diff mbox

efi/libstub/fdt: Standardize the names of EFI stub parameters

Message ID 1441874516-11364-1-git-send-email-zhaoshenglong@huawei.com
State New
Headers show

Commit Message

Shannon Zhao Sept. 10, 2015, 8:41 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

These EFI stub parameters are used to internal communication between EFI
stub and Linux kernel and EFI stub creates these parameters. But for Xen
on ARM when booting with UEFI, Xen will create a minimal DT providing
these parameters for Dom0 and Dom0 is not only Linux kernel, but also
other OS (such as FreeBSD) which will be used in the future. So here
we plan to standardize the names by dropping the prefix "linux," and
make them common to other OS. Also this will not break the compatibility
since these parameters are used to internal communication between EFI
stub and kernel.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
Look at [1] for the discussion about this in Xen ML. The purpose of this
patch is to standardize the names to make Linux ARM kernel work on Xen
with UEFI. Also it hopes other OS(e.g. FreeBSD), which will be used as
Dom0 on Xen, could support this mechanism as well.

[1]http://lists.xenproject.org/archives/html/xen-devel/2015-08/msg02250.html

 Documentation/arm/uefi.txt         | 10 +++++-----
 drivers/firmware/efi/efi.c         | 10 +++++-----
 drivers/firmware/efi/libstub/fdt.c | 10 +++++-----
 3 files changed, 15 insertions(+), 15 deletions(-)

Comments

Mark Rutland Sept. 10, 2015, 9:52 a.m. UTC | #1
Hi,

I'm not necessarily opposed to the renaming, but I think that this is
the least important thing to standardize for this to work.

On Thu, Sep 10, 2015 at 09:41:56AM +0100, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> These EFI stub parameters are used to internal communication between EFI
> stub and Linux kernel and EFI stub creates these parameters. But for Xen
> on ARM when booting with UEFI, Xen will create a minimal DT providing
> these parameters for Dom0 and Dom0 is not only Linux kernel, but also
> other OS (such as FreeBSD) which will be used in the future.

Currently the Linux EFI stub and kernel have a symbiotic relationship,
because they're intimately coupled and we don't have kexec (yet) on EFI
platforms to loosen that coupling.

If an agent other than the (kernel-specific) stub is going to provide
this to the kernel, then we need more specified than just the property
names.

That at least includes the following:

* The state of boot services (we currently have the EFI stub call
  ExitBootServices(), and I believe this is crucial to the plan for
  kexec).

* The state of the address map (we currently have the EFI stub call
  SetVirtualAddressMap()).

* The virtual address range(s) that SetVirtualAddressMap() may map
  elements to (this logic is currently in the EFI stub, and this matches
  the expectations of the kernel that it is tied to).

> So here we plan to standardize the names by dropping the prefix
> "linux," and make them common to other OS. Also this will not break
> the compatibility since these parameters are used to internal
> communication between EFI stub and kernel.

For the moment this is true, but will not be once we have kexec, so
there's a dependency (or anti-dependency) there.

> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> Look at [1] for the discussion about this in Xen ML. The purpose of this
> patch is to standardize the names to make Linux ARM kernel work on Xen
> with UEFI. Also it hopes other OS(e.g. FreeBSD), which will be used as
> Dom0 on Xen, could support this mechanism as well.
> 
> [1]http://lists.xenproject.org/archives/html/xen-devel/2015-08/msg02250.html

Per this post, it looks like to pass a DTB to the kernel Xen already
needs to know it's a Linux kernel...

I wasn't aware that there was a common standard for arm(64) kernels
other than a PE/COFF EFI application.

Does Xen not talk to EFI itself and/or give the kernel a virtual EFI
interface?

Thanks,
Mark.

>  Documentation/arm/uefi.txt         | 10 +++++-----
>  drivers/firmware/efi/efi.c         | 10 +++++-----
>  drivers/firmware/efi/libstub/fdt.c | 10 +++++-----
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/arm/uefi.txt b/Documentation/arm/uefi.txt
> index d60030a..8c83243 100644
> --- a/Documentation/arm/uefi.txt
> +++ b/Documentation/arm/uefi.txt
> @@ -45,18 +45,18 @@ following parameters:
>  ________________________________________________________________________________
>  Name                      | Size   | Description
>  ================================================================================
> -linux,uefi-system-table   | 64-bit | Physical address of the UEFI System Table.
> +uefi-system-table         | 64-bit | Physical address of the UEFI System Table.
>  --------------------------------------------------------------------------------
> -linux,uefi-mmap-start     | 64-bit | Physical address of the UEFI memory map,
> +uefi-mmap-start           | 64-bit | Physical address of the UEFI memory map,
>                            |        | populated by the UEFI GetMemoryMap() call.
>  --------------------------------------------------------------------------------
> -linux,uefi-mmap-size      | 32-bit | Size in bytes of the UEFI memory map
> +uefi-mmap-size            | 32-bit | Size in bytes of the UEFI memory map
>                            |        | pointed to in previous entry.
>  --------------------------------------------------------------------------------
> -linux,uefi-mmap-desc-size | 32-bit | Size in bytes of each entry in the UEFI
> +uefi-mmap-desc-size       | 32-bit | Size in bytes of each entry in the UEFI
>                            |        | memory map.
>  --------------------------------------------------------------------------------
> -linux,uefi-mmap-desc-ver  | 32-bit | Version of the mmap descriptor format.
> +uefi-mmap-desc-ver        | 32-bit | Version of the mmap descriptor format.
>  --------------------------------------------------------------------------------
>  linux,uefi-stub-kern-ver  | string | Copy of linux_banner from build.
>  --------------------------------------------------------------------------------
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index d6144e3..3878715 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -481,11 +481,11 @@ static __initdata struct {
>  	int offset;
>  	int size;
>  } dt_params[] = {
> -	UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
> -	UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
> -	UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
> -	UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
> -	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
> +	UEFI_PARAM("System Table", "uefi-system-table", system_table),
> +	UEFI_PARAM("MemMap Address", "uefi-mmap-start", mmap),
> +	UEFI_PARAM("MemMap Size", "uefi-mmap-size", mmap_size),
> +	UEFI_PARAM("MemMap Desc. Size", "uefi-mmap-desc-size", desc_size),
> +	UEFI_PARAM("MemMap Desc. Version", "uefi-mmap-desc-ver", desc_ver)
>  };
>  
>  struct param_info {
> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
> index ef5d764..e94589a 100644
> --- a/drivers/firmware/efi/libstub/fdt.c
> +++ b/drivers/firmware/efi/libstub/fdt.c
> @@ -118,31 +118,31 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
>  	/* Add FDT entries for EFI runtime services in chosen node. */
>  	node = fdt_subnode_offset(fdt, 0, "chosen");
>  	fdt_val64 = cpu_to_fdt64((u64)(unsigned long)sys_table);
> -	status = fdt_setprop(fdt, node, "linux,uefi-system-table",
> +	status = fdt_setprop(fdt, node, "uefi-system-table",
>  			     &fdt_val64, sizeof(fdt_val64));
>  	if (status)
>  		goto fdt_set_fail;
>  
>  	fdt_val64 = cpu_to_fdt64((u64)(unsigned long)memory_map);
> -	status = fdt_setprop(fdt, node, "linux,uefi-mmap-start",
> +	status = fdt_setprop(fdt, node, "uefi-mmap-start",
>  			     &fdt_val64,  sizeof(fdt_val64));
>  	if (status)
>  		goto fdt_set_fail;
>  
>  	fdt_val32 = cpu_to_fdt32(map_size);
> -	status = fdt_setprop(fdt, node, "linux,uefi-mmap-size",
> +	status = fdt_setprop(fdt, node, "uefi-mmap-size",
>  			     &fdt_val32,  sizeof(fdt_val32));
>  	if (status)
>  		goto fdt_set_fail;
>  
>  	fdt_val32 = cpu_to_fdt32(desc_size);
> -	status = fdt_setprop(fdt, node, "linux,uefi-mmap-desc-size",
> +	status = fdt_setprop(fdt, node, "uefi-mmap-desc-size",
>  			     &fdt_val32, sizeof(fdt_val32));
>  	if (status)
>  		goto fdt_set_fail;
>  
>  	fdt_val32 = cpu_to_fdt32(desc_ver);
> -	status = fdt_setprop(fdt, node, "linux,uefi-mmap-desc-ver",
> +	status = fdt_setprop(fdt, node, "uefi-mmap-desc-ver",
>  			     &fdt_val32, sizeof(fdt_val32));
>  	if (status)
>  		goto fdt_set_fail;
> -- 
> 2.0.4
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefano Stabellini Sept. 10, 2015, 10:19 a.m. UTC | #2
On Thu, 10 Sep 2015, Mark Rutland wrote:
> Hi,
> 
> I'm not necessarily opposed to the renaming, but I think that this is
> the least important thing to standardize for this to work.
> 
> On Thu, Sep 10, 2015 at 09:41:56AM +0100, Shannon Zhao wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > These EFI stub parameters are used to internal communication between EFI
> > stub and Linux kernel and EFI stub creates these parameters. But for Xen
> > on ARM when booting with UEFI, Xen will create a minimal DT providing
> > these parameters for Dom0 and Dom0 is not only Linux kernel, but also
> > other OS (such as FreeBSD) which will be used in the future.
> 
> Currently the Linux EFI stub and kernel have a symbiotic relationship,
> because they're intimately coupled and we don't have kexec (yet) on EFI
> platforms to loosen that coupling.
> 
> If an agent other than the (kernel-specific) stub is going to provide
> this to the kernel, then we need more specified than just the property
> names.
> 
> That at least includes the following:
> 
> * The state of boot services (we currently have the EFI stub call
>   ExitBootServices(), and I believe this is crucial to the plan for
>   kexec).
> 
> * The state of the address map (we currently have the EFI stub call
>   SetVirtualAddressMap()).
> 
> * The virtual address range(s) that SetVirtualAddressMap() may map
>   elements to (this logic is currently in the EFI stub, and this matches
>   the expectations of the kernel that it is tied to).
> 
> > So here we plan to standardize the names by dropping the prefix
> > "linux," and make them common to other OS. Also this will not break
> > the compatibility since these parameters are used to internal
> > communication between EFI stub and kernel.
> 
> For the moment this is true, but will not be once we have kexec, so
> there's a dependency (or anti-dependency) there.
> 
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> > Look at [1] for the discussion about this in Xen ML. The purpose of this
> > patch is to standardize the names to make Linux ARM kernel work on Xen
> > with UEFI. Also it hopes other OS(e.g. FreeBSD), which will be used as
> > Dom0 on Xen, could support this mechanism as well.
> > 
> > [1]http://lists.xenproject.org/archives/html/xen-devel/2015-08/msg02250.html
> 
> Per this post, it looks like to pass a DTB to the kernel Xen already
> needs to know it's a Linux kernel...
> 
> I wasn't aware that there was a common standard for arm(64) kernels
> other than a PE/COFF EFI application.
> 
> Does Xen not talk to EFI itself and/or give the kernel a virtual EFI
> interface?

Xen talks to EFI itself but the interface provided to dom0 is somewhat
different: there are no BootServices (Xen calls ExitBootServices before
running the kernel), and the RuntimeServices go via hypercalls (see
drivers/xen/efi.c).
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Sept. 10, 2015, 11:24 a.m. UTC | #3
> > Does Xen not talk to EFI itself and/or give the kernel a virtual EFI
> > interface?
> 
> Xen talks to EFI itself but the interface provided to dom0 is somewhat
> different: there are no BootServices (Xen calls ExitBootServices before
> running the kernel), and the RuntimeServices go via hypercalls (see
> drivers/xen/efi.c).

That's somewhat hideous; a non Xen-aware OS wouild presumably die if
trying to use any runtime services the normal way? I'm not keen on
describing things that the OS cannot use.

Why can't Xen give a virtual EFI interface to Dom0 / guests? e.g.
create pages of RuntimeServicesCode that are trivial assembly shims
doing hypercalls, and plumb these into the virtual EFI memory map and
tables?

That would keep things sane for any guest, allow for easy addition of
EFI features, and you could even enter the usual EFI entry point,
simulate ExitBootServices(), SetVirtualAddressMap(), and allow the guest
to make things sane for itself...

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefano Stabellini Sept. 10, 2015, 11:37 a.m. UTC | #4
On Thu, 10 Sep 2015, Mark Rutland wrote:
> > > Does Xen not talk to EFI itself and/or give the kernel a virtual EFI
> > > interface?
> > 
> > Xen talks to EFI itself but the interface provided to dom0 is somewhat
> > different: there are no BootServices (Xen calls ExitBootServices before
> > running the kernel), and the RuntimeServices go via hypercalls (see
> > drivers/xen/efi.c).
> 
> That's somewhat hideous; a non Xen-aware OS wouild presumably die if
> trying to use any runtime services the normal way? I'm not keen on
> describing things that the OS cannot use.
 
I agree that is somewhat hideous, but a non-Xen aware OS traditionally
has never been able to even boot as Dom0. On ARM it can, but it still
wouldn't be very useful (one couldn't use it to start other guests).


> Why can't Xen give a virtual EFI interface to Dom0 / guests? e.g.
> create pages of RuntimeServicesCode that are trivial assembly shims
> doing hypercalls, and plumb these into the virtual EFI memory map and
> tables?
> 
> That would keep things sane for any guest, allow for easy addition of
> EFI features, and you could even enter the usual EFI entry point,
> simulate ExitBootServices(), SetVirtualAddressMap(), and allow the guest
> to make things sane for itself...

That's the way it was done on x86 and now we have common code both in
Linux (drivers/xen/efi.c) and Xen (xen/common/efi) which implement this
scheme.  Switching to a different solution for ARM, would mean diverging
with x86, which is not nice, or reimplementing the x86 solution too,
which is expensive.

BTW I think that the idea you proposed was actually considered at the
time and deemed hard to implement, if I recall correctly.


In any case this should be separate from the shim ABI discussion.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Sept. 10, 2015, 12:15 p.m. UTC | #5
On Thu, Sep 10, 2015 at 12:37:57PM +0100, Stefano Stabellini wrote:
> On Thu, 10 Sep 2015, Mark Rutland wrote:
> > > > Does Xen not talk to EFI itself and/or give the kernel a virtual EFI
> > > > interface?
> > > 
> > > Xen talks to EFI itself but the interface provided to dom0 is somewhat
> > > different: there are no BootServices (Xen calls ExitBootServices before
> > > running the kernel), and the RuntimeServices go via hypercalls (see
> > > drivers/xen/efi.c).
> > 
> > That's somewhat hideous; a non Xen-aware OS wouild presumably die if
> > trying to use any runtime services the normal way? I'm not keen on
> > describing things that the OS cannot use.
>  
> I agree that is somewhat hideous, but a non-Xen aware OS traditionally
> has never been able to even boot as Dom0. On ARM it can, but it still
> wouldn't be very useful (one couldn't use it to start other guests).

Sure, but it feels odd to provide the usual information in this manner
if it cannot be used. If you require Xen-specific code to make things
work, I would imagine this information could be dciscovered in a
Xen-specific manner.

> > Why can't Xen give a virtual EFI interface to Dom0 / guests? e.g.
> > create pages of RuntimeServicesCode that are trivial assembly shims
> > doing hypercalls, and plumb these into the virtual EFI memory map and
> > tables?
> > 
> > That would keep things sane for any guest, allow for easy addition of
> > EFI features, and you could even enter the usual EFI entry point,
> > simulate ExitBootServices(), SetVirtualAddressMap(), and allow the guest
> > to make things sane for itself...
> 
> That's the way it was done on x86 and now we have common code both in
> Linux (drivers/xen/efi.c) and Xen (xen/common/efi) which implement this
> scheme.

This code is not currently used on arm. It might live in a location
where it may be shared, but that doesn't mean that it's common code yet.

> Switching to a different solution for ARM, would mean diverging
> with x86, which is not nice, or reimplementing the x86 solution too,
> which is expensive.
> 
> BTW I think that the idea you proposed was actually considered at the
> time and deemed hard to implement, if I recall correctly.

I appreciate that divergence is painful. We already diverge in other
respects (e.g. lack of PV page tables) because things that used to be
the case on x86 never applied to ARM.

It would be interesting to see why that was the case for x86, and
whether that applies to ARM.

> In any case this should be separate from the shim ABI discussion.

I disagree; I think this is very much relevant to the ABI discussion.
That's not to say that I insist on a particular approach, but I think
that they need to be considered together.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Jan Beulich Sept. 10, 2015, 12:55 p.m. UTC | #6
>>> On 10.09.15 at 13:37, <stefano.stabellini@eu.citrix.com> wrote:
> On Thu, 10 Sep 2015, Mark Rutland wrote:
>> Why can't Xen give a virtual EFI interface to Dom0 / guests? e.g.
>> create pages of RuntimeServicesCode that are trivial assembly shims
>> doing hypercalls, and plumb these into the virtual EFI memory map and
>> tables?
>> 
>> That would keep things sane for any guest, allow for easy addition of
>> EFI features, and you could even enter the usual EFI entry point,
>> simulate ExitBootServices(), SetVirtualAddressMap(), and allow the guest
>> to make things sane for itself...
> 
> That's the way it was done on x86 and now we have common code both in
> Linux (drivers/xen/efi.c) and Xen (xen/common/efi) which implement this
> scheme.  Switching to a different solution for ARM, would mean diverging
> with x86, which is not nice, or reimplementing the x86 solution too,
> which is expensive.
> 
> BTW I think that the idea you proposed was actually considered at the
> time and deemed hard to implement, if I recall correctly.

Considering that the EFI support is just for Dom0, and Dom0 (at
the time) had to be PV anyway, it was the more natural solution to
expose the interface via hypercalls, the more that this allows better
control over what is and primarily what is not being exposed to
Dom0. With the wrapper approach we'd be back to the same
problem (discussed elsewhere) of which EFI version to surface: The
host one would impose potentially missing extensions, while the
most recent hypervisor known one might imply hiding valuable
information from Dom0. Plus there are incompatible changes like
the altered meaning of EFI_MEMORY_WP in 2.5.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Jan Beulich Sept. 10, 2015, 1:08 p.m. UTC | #7
>>> On 10.09.15 at 14:58, <ian.campbell@citrix.com> wrote:
> On Thu, 2015-09-10 at 13:15 +0100, Mark Rutland wrote:
> 
>> > In any case this should be separate from the shim ABI discussion.
>> 
>> I disagree; I think this is very much relevant to the ABI discussion.
>> That's not to say that I insist on a particular approach, but I think
>> that they need to be considered together.
> 
> Taking a step back, the reason for using the EFI stub parameters is only
> (AFAIK) in order to be able to locate the ACPI RDSP (the root table
> pointer), which as it happens is normally passed via one of the EFI
> firmware tables.
> 
> If there was a way to achieve that goal (i.e. another way to find the RSDP)
> without opening the can of UEFI worms then we could consider that opiton
> too.
> 
> a way != the legacy x86 thing of scanning low memory of the signature, of
> course.

But even x86 doesn't do that (other than as a fallback) on EFI. The
configuration table is available to Dom0 (via XENPF_firmware_info:
XEN_FW_EFI_INFO:XEN_FW_EFI_CONFIG_TABLE).

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Ian Campbell Sept. 10, 2015, 1:30 p.m. UTC | #8
On Thu, 2015-09-10 at 07:08 -0600, Jan Beulich wrote:
> > > > On 10.09.15 at 14:58, <ian.campbell@citrix.com> wrote:
> > On Thu, 2015-09-10 at 13:15 +0100, Mark Rutland wrote:
> > 
> > > > In any case this should be separate from the shim ABI discussion.
> > > 
> > > I disagree; I think this is very much relevant to the ABI discussion.
> > > That's not to say that I insist on a particular approach, but I think
> > > that they need to be considered together.
> > 
> > Taking a step back, the reason for using the EFI stub parameters is
> > only
> > (AFAIK) in order to be able to locate the ACPI RDSP (the root table
> > pointer), which as it happens is normally passed via one of the EFI
> > firmware tables.
> > 
> > If there was a way to achieve that goal (i.e. another way to find the
> > RSDP)
> > without opening the can of UEFI worms then we could consider that
> > opiton
> > too.
> > 
> > a way != the legacy x86 thing of scanning low memory of the signature,
> > of
> > course.
> 
> But even x86 doesn't do that (other than as a fallback) on EFI.

Indeed, I was referring legacy (non-EFI) mode.

>  The
> configuration table is available to Dom0 (via XENPF_firmware_info:
> XEN_FW_EFI_INFO:XEN_FW_EFI_CONFIG_TABLE).

Under ARM we find out we are running under Xen from the ACPI tables, so
there is a chicken and egg situation there.

Not insoluble I'm sure, if we want to go down this route.
Ian.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefano Stabellini Sept. 10, 2015, 1:52 p.m. UTC | #9
On Thu, 10 Sep 2015, Mark Rutland wrote:
> On Thu, Sep 10, 2015 at 12:37:57PM +0100, Stefano Stabellini wrote:
> > On Thu, 10 Sep 2015, Mark Rutland wrote:
> > > > > Does Xen not talk to EFI itself and/or give the kernel a virtual EFI
> > > > > interface?
> > > > 
> > > > Xen talks to EFI itself but the interface provided to dom0 is somewhat
> > > > different: there are no BootServices (Xen calls ExitBootServices before
> > > > running the kernel), and the RuntimeServices go via hypercalls (see
> > > > drivers/xen/efi.c).
> > > 
> > > That's somewhat hideous; a non Xen-aware OS wouild presumably die if
> > > trying to use any runtime services the normal way? I'm not keen on
> > > describing things that the OS cannot use.
> >  
> > I agree that is somewhat hideous, but a non-Xen aware OS traditionally
> > has never been able to even boot as Dom0. On ARM it can, but it still
> > wouldn't be very useful (one couldn't use it to start other guests).
> 
> Sure, but it feels odd to provide the usual information in this manner
> if it cannot be used. If you require Xen-specific code to make things
> work, I would imagine this information could be dciscovered in a
> Xen-specific manner.

We need ACPI (or Device Tree) to find that Xen is available on the
platform, so we cannot use Xen-specific code to get the ACPI tables.


> > > Why can't Xen give a virtual EFI interface to Dom0 / guests? e.g.
> > > create pages of RuntimeServicesCode that are trivial assembly shims
> > > doing hypercalls, and plumb these into the virtual EFI memory map and
> > > tables?
> > > 
> > > That would keep things sane for any guest, allow for easy addition of
> > > EFI features, and you could even enter the usual EFI entry point,
> > > simulate ExitBootServices(), SetVirtualAddressMap(), and allow the guest
> > > to make things sane for itself...
> > 
> > That's the way it was done on x86 and now we have common code both in
> > Linux (drivers/xen/efi.c) and Xen (xen/common/efi) which implement this
> > scheme.
> 
> This code is not currently used on arm. It might live in a location
> where it may be shared, but that doesn't mean that it's common code yet.

Yeah, but that was clearly the intention.


> > Switching to a different solution for ARM, would mean diverging
> > with x86, which is not nice, or reimplementing the x86 solution too,
> > which is expensive.
> > 
> > BTW I think that the idea you proposed was actually considered at the
> > time and deemed hard to implement, if I recall correctly.
> 
> I appreciate that divergence is painful. We already diverge in other
> respects (e.g. lack of PV page tables) because things that used to be
> the case on x86 never applied to ARM.
> 
> It would be interesting to see why that was the case for x86, and
> whether that applies to ARM.

I have been a big advocate of doing things differently, and more
cleanly, on ARM, but in this case the code is already non-arch specific.
We are not talking about pagetables, which of course cannot be reused
as-is even if we wanted to.

For once, I think we should follow the x86 way, for convenience and for
the reasons brought up by Jan.


> > In any case this should be separate from the shim ABI discussion.
> 
> I disagree; I think this is very much relevant to the ABI discussion.
> That's not to say that I insist on a particular approach, but I think
> that they need to be considered together.

Let's suppose Xen didn't expose any RuntimeServices at all, would that
make it easier to discuss about the EFI stub parameters?  In the grant
scheme of things, they are not that important, as Ian wrote what is
important is how to pass the RSDP.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leif Lindholm Sept. 10, 2015, 2:13 p.m. UTC | #10
On Thu, Sep 10, 2015 at 02:52:25PM +0100, Stefano Stabellini wrote:
> > > In any case this should be separate from the shim ABI discussion.
> > 
> > I disagree; I think this is very much relevant to the ABI discussion.
> > That's not to say that I insist on a particular approach, but I think
> > that they need to be considered together.
> 
> Let's suppose Xen didn't expose any RuntimeServices at all, would that
> make it easier to discuss about the EFI stub parameters?

Possibly :)

> In the grant
> scheme of things, they are not that important, as Ian wrote what is
> important is how to pass the RSDP.

So, we have discussed in the past having the ability to get at
configuration tables when UEFI is not available. Say, for example,
that we wanted SMBIOS support on a platform with U-Boot firmware.

Since all that is needed then is a UEFI System Table with a pointer to
a configuration table array, this should be fairly straightforward to
implement statically. The other parameters would not be necessary.

It would however require minor changes to the arm64 kernel UEFI support.

/
    Leif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Mark Rutland Sept. 10, 2015, 2:49 p.m. UTC | #11
On Thu, Sep 10, 2015 at 02:52:25PM +0100, Stefano Stabellini wrote:
> On Thu, 10 Sep 2015, Mark Rutland wrote:
> > On Thu, Sep 10, 2015 at 12:37:57PM +0100, Stefano Stabellini wrote:
> > > On Thu, 10 Sep 2015, Mark Rutland wrote:
> > > > > > Does Xen not talk to EFI itself and/or give the kernel a virtual EFI
> > > > > > interface?
> > > > > 
> > > > > Xen talks to EFI itself but the interface provided to dom0 is somewhat
> > > > > different: there are no BootServices (Xen calls ExitBootServices before
> > > > > running the kernel), and the RuntimeServices go via hypercalls (see
> > > > > drivers/xen/efi.c).
> > > > 
> > > > That's somewhat hideous; a non Xen-aware OS wouild presumably die if
> > > > trying to use any runtime services the normal way? I'm not keen on
> > > > describing things that the OS cannot use.
> > >  
> > > I agree that is somewhat hideous, but a non-Xen aware OS traditionally
> > > has never been able to even boot as Dom0. On ARM it can, but it still
> > > wouldn't be very useful (one couldn't use it to start other guests).
> > 
> > Sure, but it feels odd to provide the usual information in this manner
> > if it cannot be used. If you require Xen-specific code to make things
> > work, I would imagine this information could be dciscovered in a
> > Xen-specific manner.
> 
> We need ACPI (or Device Tree) to find that Xen is available on the
> platform, so we cannot use Xen-specific code to get the ACPI tables.

I don't understand. The proposition already involves passing a custom DT
to the OS, implying that Xen knows how to boot that OS, and how to pass
it a DTB.

Consider:

A) In the DT-only case, we go:

   DT -> Discover Xen -> Xen-specific stuff


B) The proposition is that un the ACPI case we go:

   DT -> EFI tables -> ACPI tables -> Discover Xen -> Xen-specific stuff -> override EFI/ACPI stuff
         \-----------------------/
          (be really cautious here)


C) When you could go:

   DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI discovery


D) If you want to be generic:
   EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific stuff
          \------------------------------------------/
	   (virtualize these, provide shims to Dom0, but handle
	    everything in Xen itself)


E) Partially-generic option:
   EFI -> EFI application -> Xen detected by registered GUID -> Xen-specific EFI bootloader stuff -> OS in Xen-specific configuration


> > > In any case this should be separate from the shim ABI discussion.
> > 
> > I disagree; I think this is very much relevant to the ABI discussion.
> > That's not to say that I insist on a particular approach, but I think
> > that they need to be considered together.
> 
> Let's suppose Xen didn't expose any RuntimeServices at all, would that
> make it easier to discuss about the EFI stub parameters?

It would simply the protocol specific to Xen, certainly.

> In the grant scheme of things, they are not that important, as Ian
> wrote what is important is how to pass the RSDP.

Unfortunately we're still going to have to care about this eventually,
even if for something like kexec. So we still need to spec out the state
of things if this is going to be truly generic.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Sept. 10, 2015, 2:53 p.m. UTC | #12
On Thu, Sep 10, 2015 at 01:55:25PM +0100, Jan Beulich wrote:
> >>> On 10.09.15 at 13:37, <stefano.stabellini@eu.citrix.com> wrote:
> > On Thu, 10 Sep 2015, Mark Rutland wrote:
> >> Why can't Xen give a virtual EFI interface to Dom0 / guests? e.g.
> >> create pages of RuntimeServicesCode that are trivial assembly shims
> >> doing hypercalls, and plumb these into the virtual EFI memory map and
> >> tables?
> >> 
> >> That would keep things sane for any guest, allow for easy addition of
> >> EFI features, and you could even enter the usual EFI entry point,
> >> simulate ExitBootServices(), SetVirtualAddressMap(), and allow the guest
> >> to make things sane for itself...
> > 
> > That's the way it was done on x86 and now we have common code both in
> > Linux (drivers/xen/efi.c) and Xen (xen/common/efi) which implement this
> > scheme.  Switching to a different solution for ARM, would mean diverging
> > with x86, which is not nice, or reimplementing the x86 solution too,
> > which is expensive.
> > 
> > BTW I think that the idea you proposed was actually considered at the
> > time and deemed hard to implement, if I recall correctly.
> 
> Considering that the EFI support is just for Dom0, and Dom0 (at
> the time) had to be PV anyway, it was the more natural solution to
> expose the interface via hypercalls, the more that this allows better
> control over what is and primarily what is not being exposed to
> Dom0. With the wrapper approach we'd be back to the same
> problem (discussed elsewhere) of which EFI version to surface: The
> host one would impose potentially missing extensions, while the
> most recent hypervisor known one might imply hiding valuable
> information from Dom0. Plus there are incompatible changes like
> the altered meaning of EFI_MEMORY_WP in 2.5.

I'm not sure I follow how hypercalls solve any impedance mismatch here;
you're still expecting Dom0 to call up to Xen in order to perform calls,
and all I suggested was a different location for those hypercalls.

If Xen is happy to make such calls blindly, why does it matter if the
hypercall was in the kernel binary or an external shim?

Incompatible changes are a spec problem regardless of how this is
handled.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Beulich Sept. 10, 2015, 3:06 p.m. UTC | #13
>>> On 10.09.15 at 16:53, <mark.rutland@arm.com> wrote:
> On Thu, Sep 10, 2015 at 01:55:25PM +0100, Jan Beulich wrote:
>> >>> On 10.09.15 at 13:37, <stefano.stabellini@eu.citrix.com> wrote:
>> > On Thu, 10 Sep 2015, Mark Rutland wrote:
>> >> Why can't Xen give a virtual EFI interface to Dom0 / guests? e.g.
>> >> create pages of RuntimeServicesCode that are trivial assembly shims
>> >> doing hypercalls, and plumb these into the virtual EFI memory map and
>> >> tables?
>> >> 
>> >> That would keep things sane for any guest, allow for easy addition of
>> >> EFI features, and you could even enter the usual EFI entry point,
>> >> simulate ExitBootServices(), SetVirtualAddressMap(), and allow the guest
>> >> to make things sane for itself...
>> > 
>> > That's the way it was done on x86 and now we have common code both in
>> > Linux (drivers/xen/efi.c) and Xen (xen/common/efi) which implement this
>> > scheme.  Switching to a different solution for ARM, would mean diverging
>> > with x86, which is not nice, or reimplementing the x86 solution too,
>> > which is expensive.
>> > 
>> > BTW I think that the idea you proposed was actually considered at the
>> > time and deemed hard to implement, if I recall correctly.
>> 
>> Considering that the EFI support is just for Dom0, and Dom0 (at
>> the time) had to be PV anyway, it was the more natural solution to
>> expose the interface via hypercalls, the more that this allows better
>> control over what is and primarily what is not being exposed to
>> Dom0. With the wrapper approach we'd be back to the same
>> problem (discussed elsewhere) of which EFI version to surface: The
>> host one would impose potentially missing extensions, while the
>> most recent hypervisor known one might imply hiding valuable
>> information from Dom0. Plus there are incompatible changes like
>> the altered meaning of EFI_MEMORY_WP in 2.5.
> 
> I'm not sure I follow how hypercalls solve any impedance mismatch here;
> you're still expecting Dom0 to call up to Xen in order to perform calls,
> and all I suggested was a different location for those hypercalls.
> 
> If Xen is happy to make such calls blindly, why does it matter if the
> hypercall was in the kernel binary or an external shim?

Because there could be new entries in SystemTable->RuntimeServices
(expected and blindly but validly called by the kernel). Even worse
(because likely harder to deal with) would be new fields in other
structures.

> Incompatible changes are a spec problem regardless of how this is
> handled.

Not necessarily - we don't expose the memory map (we'd have to
if we were to mimic EFI for Dom0), and hence the mentioned issue
doesn't exist in our model.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Stefano Stabellini Sept. 10, 2015, 4:10 p.m. UTC | #14
On Thu, 10 Sep 2015, Mark Rutland wrote:
> On Thu, Sep 10, 2015 at 02:52:25PM +0100, Stefano Stabellini wrote:
> > On Thu, 10 Sep 2015, Mark Rutland wrote:
> > > On Thu, Sep 10, 2015 at 12:37:57PM +0100, Stefano Stabellini wrote:
> > > > On Thu, 10 Sep 2015, Mark Rutland wrote:
> > > > > > > Does Xen not talk to EFI itself and/or give the kernel a virtual EFI
> > > > > > > interface?
> > > > > > 
> > > > > > Xen talks to EFI itself but the interface provided to dom0 is somewhat
> > > > > > different: there are no BootServices (Xen calls ExitBootServices before
> > > > > > running the kernel), and the RuntimeServices go via hypercalls (see
> > > > > > drivers/xen/efi.c).
> > > > > 
> > > > > That's somewhat hideous; a non Xen-aware OS wouild presumably die if
> > > > > trying to use any runtime services the normal way? I'm not keen on
> > > > > describing things that the OS cannot use.
> > > >  
> > > > I agree that is somewhat hideous, but a non-Xen aware OS traditionally
> > > > has never been able to even boot as Dom0. On ARM it can, but it still
> > > > wouldn't be very useful (one couldn't use it to start other guests).
> > > 
> > > Sure, but it feels odd to provide the usual information in this manner
> > > if it cannot be used. If you require Xen-specific code to make things
> > > work, I would imagine this information could be dciscovered in a
> > > Xen-specific manner.
> > 
> > We need ACPI (or Device Tree) to find that Xen is available on the
> > platform, so we cannot use Xen-specific code to get the ACPI tables.
> 
> I don't understand. The proposition already involves passing a custom DT
> to the OS, implying that Xen knows how to boot that OS, and how to pass
> it a DTB.
> 
> Consider:
> 
> A) In the DT-only case, we go:
> 
>    DT -> Discover Xen -> Xen-specific stuff
> 
> 
> B) The proposition is that un the ACPI case we go:
> 
>    DT -> EFI tables -> ACPI tables -> Discover Xen -> Xen-specific stuff -> override EFI/ACPI stuff
>          \-----------------------/
>           (be really cautious here)

Well, yes. To be pedantic "override" here would just be the different
delivery method for RuntimeServices. I guess it still counts.


> C) When you could go:
> 
>    DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI discovery

I take you mean discovering Xen with the usual Xen hypervisor node on
device tree. I think that C) is a good option actually. I like it. Not
sure why we didn't think about this earlier. Is there anything EFI or
ACPI which is needed before Xen support is discovered by
arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()?

If not, we could just go for this. A lot of complexity would go away.


> D) If you want to be generic:
>    EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific stuff
>           \------------------------------------------/
> 	   (virtualize these, provide shims to Dom0, but handle
> 	    everything in Xen itself)

I think that this is good in theory but could turn out to be a lot of
work in practice. We could probably virtualize the RuntimeServices but
the BootServices are troublesome.


> 
> E) Partially-generic option:
>    EFI -> EFI application -> Xen detected by registered GUID -> Xen-specific EFI bootloader stuff -> OS in Xen-specific configuration
> 
> 
> > > > In any case this should be separate from the shim ABI discussion.
> > > 
> > > I disagree; I think this is very much relevant to the ABI discussion.
> > > That's not to say that I insist on a particular approach, but I think
> > > that they need to be considered together.
> > 
> > Let's suppose Xen didn't expose any RuntimeServices at all, would that
> > make it easier to discuss about the EFI stub parameters?
> 
> It would simply the protocol specific to Xen, certainly.
> 
> > In the grant scheme of things, they are not that important, as Ian
> > wrote what is important is how to pass the RSDP.
> 
> Unfortunately we're still going to have to care about this eventually,
> even if for something like kexec. So we still need to spec out the state
> of things if this is going to be truly generic.
 
Fair enough. My position is that if we restrict this to RuntimeServices,
it might be possible, but I still prefer C).
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Sept. 10, 2015, 4:23 p.m. UTC | #15
> > C) When you could go:
> > 
> >    DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI discovery
> 
> I take you mean discovering Xen with the usual Xen hypervisor node on
> device tree. I think that C) is a good option actually. I like it. Not
> sure why we didn't think about this earlier. Is there anything EFI or
> ACPI which is needed before Xen support is discovered by
> arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()?

Currently lots (including the memory map). With the stuff to support
SPCR, the ACPI discovery would be moved before xen_early_init().

> If not, we could just go for this. A lot of complexity would go away.

I suspect this would still be fairly complex, but would at least prevent
the Xen-specific EFI handling from adversely affecting the native case.

> > D) If you want to be generic:
> >    EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific stuff
> >           \------------------------------------------/
> > 	   (virtualize these, provide shims to Dom0, but handle
> > 	    everything in Xen itself)
> 
> I think that this is good in theory but could turn out to be a lot of
> work in practice. We could probably virtualize the RuntimeServices but
> the BootServices are troublesome.

What's troublesome with the boot services?

What can't be simulated?

> > E) Partially-generic option:
> >    EFI -> EFI application -> Xen detected by registered GUID -> Xen-specific EFI bootloader stuff -> OS in Xen-specific configuration
> > 
> > 
> > > > > In any case this should be separate from the shim ABI discussion.
> > > > 
> > > > I disagree; I think this is very much relevant to the ABI discussion.
> > > > That's not to say that I insist on a particular approach, but I think
> > > > that they need to be considered together.
> > > 
> > > Let's suppose Xen didn't expose any RuntimeServices at all, would that
> > > make it easier to discuss about the EFI stub parameters?
> > 
> > It would simply the protocol specific to Xen, certainly.
> > 
> > > In the grant scheme of things, they are not that important, as Ian
> > > wrote what is important is how to pass the RSDP.
> > 
> > Unfortunately we're still going to have to care about this eventually,
> > even if for something like kexec. So we still need to spec out the state
> > of things if this is going to be truly generic.
>  
> Fair enough. My position is that if we restrict this to RuntimeServices,
> it might be possible, but I still prefer C).

Regardless of what we do we still need a well-defined state here, which
brings us back to the initial problem eventually.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Ian Campbell Sept. 11, 2015, 11 a.m. UTC | #16
On Thu, 2015-09-10 at 17:10 +0100, Stefano Stabellini wrote:

> > C) When you could go:
> > 
> >    DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI
> > discovery
> 
> I take you mean discovering Xen with the usual Xen hypervisor node on
> device tree.

There may be other options, such as ARM defining an architectural mechanism
to do early detection of hypervisors e.g. by defining some bit in some
hypervisor controllable register (MPIDR?) to say "a hypervisor is present"
and defining an hvc or smc call which can be used to ask which one it is.

Ian.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Kiper Sept. 11, 2015, 12:46 p.m. UTC | #17
On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote:
> > > C) When you could go:
> > >
> > >    DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI discovery
> >
> > I take you mean discovering Xen with the usual Xen hypervisor node on
> > device tree. I think that C) is a good option actually. I like it. Not
> > sure why we didn't think about this earlier. Is there anything EFI or
> > ACPI which is needed before Xen support is discovered by
> > arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()?
>
> Currently lots (including the memory map). With the stuff to support
> SPCR, the ACPI discovery would be moved before xen_early_init().
>
> > If not, we could just go for this. A lot of complexity would go away.
>
> I suspect this would still be fairly complex, but would at least prevent
> the Xen-specific EFI handling from adversely affecting the native case.
>
> > > D) If you want to be generic:
> > >    EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific stuff
> > >           \------------------------------------------/
> > > 	   (virtualize these, provide shims to Dom0, but handle
> > > 	    everything in Xen itself)
> >
> > I think that this is good in theory but could turn out to be a lot of
> > work in practice. We could probably virtualize the RuntimeServices but
> > the BootServices are troublesome.
>
> What's troublesome with the boot services?
>
> What can't be simulated?

How do you want to access bare metal EFI boot services from dom0 if they
were shutdown long time ago before loading dom0 image? What do you need
from EFI boot services in dom0?

Daniel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stefano Stabellini Sept. 11, 2015, 1:14 p.m. UTC | #18
On Fri, 11 Sep 2015, Daniel Kiper wrote:
> On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote:
> > > > C) When you could go:
> > > >
> > > >    DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI discovery
> > >
> > > I take you mean discovering Xen with the usual Xen hypervisor node on
> > > device tree. I think that C) is a good option actually. I like it. Not
> > > sure why we didn't think about this earlier. Is there anything EFI or
> > > ACPI which is needed before Xen support is discovered by
> > > arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()?
> >
> > Currently lots (including the memory map). With the stuff to support
> > SPCR, the ACPI discovery would be moved before xen_early_init().
> >
> > > If not, we could just go for this. A lot of complexity would go away.
> >
> > I suspect this would still be fairly complex, but would at least prevent
> > the Xen-specific EFI handling from adversely affecting the native case.
> >
> > > > D) If you want to be generic:
> > > >    EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific stuff
> > > >           \------------------------------------------/
> > > > 	   (virtualize these, provide shims to Dom0, but handle
> > > > 	    everything in Xen itself)
> > >
> > > I think that this is good in theory but could turn out to be a lot of
> > > work in practice. We could probably virtualize the RuntimeServices but
> > > the BootServices are troublesome.
> >
> > What's troublesome with the boot services?
> >
> > What can't be simulated?
> 
> How do you want to access bare metal EFI boot services from dom0 if they
> were shutdown long time ago before loading dom0 image? What do you need
> from EFI boot services in dom0?

That's right. Trying to emulate BootServices after the real
ExitBootServices has already been called seems like a very bad plan.

I think that whatever interface we come up with, would need to be past
ExitBootServices.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Sept. 11, 2015, 1:30 p.m. UTC | #19
On 11 September 2015 at 15:14, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Fri, 11 Sep 2015, Daniel Kiper wrote:
>> On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote:
>> > > > C) When you could go:
>> > > >
>> > > >    DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI discovery
>> > >
>> > > I take you mean discovering Xen with the usual Xen hypervisor node on
>> > > device tree. I think that C) is a good option actually. I like it. Not
>> > > sure why we didn't think about this earlier. Is there anything EFI or
>> > > ACPI which is needed before Xen support is discovered by
>> > > arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()?
>> >
>> > Currently lots (including the memory map). With the stuff to support
>> > SPCR, the ACPI discovery would be moved before xen_early_init().
>> >
>> > > If not, we could just go for this. A lot of complexity would go away.
>> >
>> > I suspect this would still be fairly complex, but would at least prevent
>> > the Xen-specific EFI handling from adversely affecting the native case.
>> >
>> > > > D) If you want to be generic:
>> > > >    EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific stuff
>> > > >           \------------------------------------------/
>> > > >            (virtualize these, provide shims to Dom0, but handle
>> > > >             everything in Xen itself)
>> > >
>> > > I think that this is good in theory but could turn out to be a lot of
>> > > work in practice. We could probably virtualize the RuntimeServices but
>> > > the BootServices are troublesome.
>> >
>> > What's troublesome with the boot services?
>> >
>> > What can't be simulated?
>>
>> How do you want to access bare metal EFI boot services from dom0 if they
>> were shutdown long time ago before loading dom0 image? What do you need
>> from EFI boot services in dom0?
>
> That's right. Trying to emulate BootServices after the real
> ExitBootServices has already been called seems like a very bad plan.
>
> I think that whatever interface we come up with, would need to be past
> ExitBootServices.

It feels like this discussion is going in circles.

When we discussed this six months ago, we already concluded that,
since UEFI is the only specified way that the presence of ACPI is
advertised on an ARM system, we need to emulate UEFI to some extent.

So we need the EFI system table to expose the UEFI configuration table
that carries the ACPI root pointer.

Since ACPI support also relies on the UEFI memory map (I think?), we
need that as well.

These two items are exactly what we pass via the UEFI DT properties,
so we should indeed promote the current de-facto binding to a proper
binding, and renaming the properties makes sense in that context.

I agree that this should also include a description of the expected
state of the firmware, i.e., that ExitBootServices() has been called,
and that the memory map has been populated with virtual address, which
have been installed using SetVirtualAddressMap() if they differ from
the physical addresses. (The current implementation on the kernel side
is perfectly capable of dealing with a 1:1 mapping).

Beyond that, there is no point in pretending to be a full UEFI
implementation, imo. Boot services are not required, nor are runtime
services (only the current EFI init code on arm needs to be modified
to deal with a NULL runtime services pointer)
Daniel Kiper Sept. 11, 2015, 3:45 p.m. UTC | #20
On Fri, Sep 11, 2015 at 03:30:15PM +0200, Ard Biesheuvel wrote:
> On 11 September 2015 at 15:14, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> > On Fri, 11 Sep 2015, Daniel Kiper wrote:
> >> On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote:
> >> > > > C) When you could go:
> >> > > >
> >> > > >    DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI discovery
> >> > >
> >> > > I take you mean discovering Xen with the usual Xen hypervisor node on
> >> > > device tree. I think that C) is a good option actually. I like it. Not
> >> > > sure why we didn't think about this earlier. Is there anything EFI or
> >> > > ACPI which is needed before Xen support is discovered by
> >> > > arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()?
> >> >
> >> > Currently lots (including the memory map). With the stuff to support
> >> > SPCR, the ACPI discovery would be moved before xen_early_init().
> >> >
> >> > > If not, we could just go for this. A lot of complexity would go away.
> >> >
> >> > I suspect this would still be fairly complex, but would at least prevent
> >> > the Xen-specific EFI handling from adversely affecting the native case.
> >> >
> >> > > > D) If you want to be generic:
> >> > > >    EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific stuff
> >> > > >           \------------------------------------------/
> >> > > >            (virtualize these, provide shims to Dom0, but handle
> >> > > >             everything in Xen itself)
> >> > >
> >> > > I think that this is good in theory but could turn out to be a lot of
> >> > > work in practice. We could probably virtualize the RuntimeServices but
> >> > > the BootServices are troublesome.
> >> >
> >> > What's troublesome with the boot services?
> >> >
> >> > What can't be simulated?
> >>
> >> How do you want to access bare metal EFI boot services from dom0 if they
> >> were shutdown long time ago before loading dom0 image? What do you need
> >> from EFI boot services in dom0?
> >
> > That's right. Trying to emulate BootServices after the real
> > ExitBootServices has already been called seems like a very bad plan.
> >
> > I think that whatever interface we come up with, would need to be past
> > ExitBootServices.
>
> It feels like this discussion is going in circles.
>
> When we discussed this six months ago, we already concluded that,
> since UEFI is the only specified way that the presence of ACPI is
> advertised on an ARM system, we need to emulate UEFI to some extent.
>
> So we need the EFI system table to expose the UEFI configuration table
> that carries the ACPI root pointer.
>
> Since ACPI support also relies on the UEFI memory map (I think?), we
> need that as well.
>
> These two items are exactly what we pass via the UEFI DT properties,
> so we should indeed promote the current de-facto binding to a proper
> binding, and renaming the properties makes sense in that context.
>
> I agree that this should also include a description of the expected
> state of the firmware, i.e., that ExitBootServices() has been called,
> and that the memory map has been populated with virtual address, which
> have been installed using SetVirtualAddressMap() if they differ from
> the physical addresses. (The current implementation on the kernel side
> is perfectly capable of dealing with a 1:1 mapping).
>
> Beyond that, there is no point in pretending to be a full UEFI
> implementation, imo. Boot services are not required, nor are runtime
> services (only the current EFI init code on arm needs to be modified
> to deal with a NULL runtime services pointer)

Taking into account above I think that you have most of the code in place.
Please take a look at linux/arch/x86/xen/efi.c, linux/drivers/acpi/osl.c
and linux/drivers/xen/efi.c (maybe somewhere else). In general you should
create ARM version of xen_efi_init() (x86 version you can find in
linux/drivers/xen/efi.c; it is very simple thing), maybe add some
code in a few places and voila.

Daniel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Sept. 11, 2015, 4:33 p.m. UTC | #21
> It feels like this discussion is going in circles.
> 
> When we discussed this six months ago, we already concluded that,
> since UEFI is the only specified way that the presence of ACPI is
> advertised on an ARM system, we need to emulate UEFI to some extent.

My understanding from the last time I was present at such a discussion
was that the emulation was complete from the kernel's PoV (i.e. no
special case handling was required). 

Evidently I misunderstood.

One of the main points of rationale for requiring EFI was that we'd have
a well-defined system state as per the EFI (and ACPI) standards. We'd
know we had the EFI memory map, services, etc (with the memory map and
configuration tables being the most important elements). We didn't want
to have to try to reconcile a DT memory map and ACPI, for instance.

That is somewhat (though admitedly not entirely) broken if we require a
set of rules to be applied beyond what the standards mandate.  That is
broken if we require a non-standard set of rules to be applied, and
limits what we can do in the !Xen case.

> So we need the EFI system table to expose the UEFI configuration table
> that carries the ACPI root pointer.
> 
> Since ACPI support also relies on the UEFI memory map (I think?), we
> need that as well.
> 
> These two items are exactly what we pass via the UEFI DT properties,
> so we should indeed promote the current de-facto binding to a proper
> binding, and renaming the properties makes sense in that context.

I agree that we need to sort these out.

> I agree that this should also include a description of the expected
> state of the firmware, i.e., that ExitBootServices() has been called,
> and that the memory map has been populated with virtual address, which
> have been installed using SetVirtualAddressMap() if they differ from
> the physical addresses. (The current implementation on the kernel side
> is perfectly capable of dealing with a 1:1 mapping).
> 
> Beyond that, there is no point in pretending to be a full UEFI
> implementation, imo. Boot services are not required, nor are runtime
> services (only the current EFI init code on arm needs to be modified
> to deal with a NULL runtime services pointer)

I'm not keen on this because it involves applying Xen-specific caveats
atop of what the UEFI spec says (e.g. as runtime services might be NULL
under Xen). As the spec and Xen evolve, those caveats shift, and that's
going to be fragile for all users regardleses of Xen.

If Xen needs special-casing, then I'd rather that Xen were detected
first and provided us with what it knows is safe for us to use, rather
than we tip-toe around until we're sure Xen isn't present and/or doesn't
need additional constraints met.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Sept. 11, 2015, 4:36 p.m. UTC | #22
> >> Considering that the EFI support is just for Dom0, and Dom0 (at
> >> the time) had to be PV anyway, it was the more natural solution to
> >> expose the interface via hypercalls, the more that this allows better
> >> control over what is and primarily what is not being exposed to
> >> Dom0. With the wrapper approach we'd be back to the same
> >> problem (discussed elsewhere) of which EFI version to surface: The
> >> host one would impose potentially missing extensions, while the
> >> most recent hypervisor known one might imply hiding valuable
> >> information from Dom0. Plus there are incompatible changes like
> >> the altered meaning of EFI_MEMORY_WP in 2.5.
> > 
> > I'm not sure I follow how hypercalls solve any impedance mismatch here;
> > you're still expecting Dom0 to call up to Xen in order to perform calls,
> > and all I suggested was a different location for those hypercalls.
> > 
> > If Xen is happy to make such calls blindly, why does it matter if the
> > hypercall was in the kernel binary or an external shim?
> 
> Because there could be new entries in SystemTable->RuntimeServices
> (expected and blindly but validly called by the kernel). Even worse
> (because likely harder to deal with) would be new fields in other
> structures.

Any of these could cause Xen to blow up, while Xen could always provide
a known-safe (but potentially sub-optimal) view to the kernel by
default.

> > Incompatible changes are a spec problem regardless of how this is
> > handled.
> 
> Not necessarily - we don't expose the memory map (we'd have to
> if we were to mimic EFI for Dom0), and hence the mentioned issue
> doesn't exist in our model.

We have to expose _some_ memory map, so I don't follow this point.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Sept. 14, 2015, 9:09 a.m. UTC | #23
On 14 September 2015 at 10:42, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
[..]

>
> It only needs to apply following patch to fix a bug in Linux kernel when
> mapping EFI_MEMORY_RUNTIME memory.
>

Could you explain why you think efi_virtmap_init() should fail if
there are no EFI_MEMORY_RUNTIME regions?

The absence of such regions is allowed by the spec, so
efi_virtmap_init() is correct imo to return success.

If you are trying to work around the issue where Xen does not expose
any Runtime Services regions, there is simply no way to do that and be
still UEFI compliant. I have suggested before that we should perhaps
tolerate this anyway, by considering the case where the EFI System
Table has a NULL runtime services pointer. But rigging
efi_virtmap_init() like this is really not the way to achieve that.
Ard Biesheuvel Sept. 14, 2015, 9:36 a.m. UTC | #24
(snip some cc's)

On 14 September 2015 at 11:31, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>
>
> On 2015/9/14 17:09, Ard Biesheuvel wrote:
>> On 14 September 2015 at 10:42, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>> [..]
>>
>>>
>>> It only needs to apply following patch to fix a bug in Linux kernel when
>>> mapping EFI_MEMORY_RUNTIME memory.
>>>
>>
>> Could you explain why you think efi_virtmap_init() should fail if
>> there are no EFI_MEMORY_RUNTIME regions?
>>
>
> My understanding is that if there are no EFI_MEMORY_RUNTIME regions, it
> means we can't use runtime services and should not set the bit
> EFI_RUNTIME_SERVICES of efi.flags. But if efi_virtmap_init() return
> true, the bit will be set.
>

As I said, if you don't want the EFI_RUNTIME_SERVICES bit to be set
for other reasons, don't rig efi_virtmap_init() to return false when
it shouldn't.

>> The absence of such regions is allowed by the spec, so
>> efi_virtmap_init() is correct imo to return success.
>>
> Sorry, not well know about the spec. Could you point out where the spec
> says this?
>

Well, I think it doesn't work that way. You are claiming that a memory
map without at least one EFI_MEMORY_RUNTIME constitutes an error
condition, so the burden is on you to provide a reference to the spec
that says you must have at least one such region.
Ard Biesheuvel Sept. 14, 2015, 9:43 a.m. UTC | #25
On 14 September 2015 at 11:25, Mark Rutland <mark.rutland@arm.com> wrote:
> On Sat, Sep 12, 2015 at 12:36:55PM +0100, Daniel Kiper wrote:
>> On Fri, Sep 11, 2015 at 05:25:59PM +0100, Mark Rutland wrote:
>> > On Fri, Sep 11, 2015 at 01:46:43PM +0100, Daniel Kiper wrote:
>> > > On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote:
>>
>> [...]
>>
>> > > > What's troublesome with the boot services?
>> > > >
>> > > > What can't be simulated?
>> > >
>> > > How do you want to access bare metal EFI boot services from dom0 if they
>> > > were shutdown long time ago before loading dom0 image?
>> >
>> > I don't want to.
>> >
>> > I asked "What can't be simulated?" because I assumed everything
>> > necessary/mandatory could be simulated without needinng access to any
>> > real EFI boot services.
>> >
>> > As far as I can see all that's necessary is to provide a compatible
>> > interface.
>>
>> Could you be more precise what do you need? Please enumerate. UEFI spec has
>> more than 2500 pages and I do not think that we need all stuff in dom0.
>>
>> > > What do you need from EFI boot services in dom0?
>> >
>> > The ability to call ExitBootServices() and SetVirtualAddressMap() on a
>> > _virtual_ address map for _virtual_ services provided by the hypervisor.
>>
>> I am confused. Why do you need that? Please remember, EFI is owned and
>> operated by Xen hypervisor. dom0 does not have direct access to EFI.
>
> Let's take a step back.
>
> My objection here is to passing the Dom0 kernel properties as if it were
> booted with direct access to a full UEFI, then later fixing that up
> (when Xen is detected and we apply its hypercall EFI implementation).
>

To be honest, I don't think that has ever been suggested here. What
was suggested is to provide a minimal EFI like environment that allows
the post-stub EFI code to proceed and find the ACPI root pointer.

> If the kernel cannot use EFI natively, why pretend to the kernel that it
> can? The hypercall implementation is _not_ EFI (though it provides
> access to some services).
>

To get access to the ACPI root pointer, for which there is only one
specified way of obtaining it on ARM, which is via the UEFI
configuration table database

> The two ways I can see providing Dom0 with EFI services are:
>
> * Have Xen create shims for any services, in which any hypercalls live,
>   and pass these to the kernel with a virtual system table. This keeps
>   the interface to the kernel the same regardless of Xen.
>
> * Have the kernel detect Xen EFI capability via Xen, without passing the
>   usual native EFI parameters. This can then be installed into the
>   kernel in a Xen-specific manner, and we know from the outset that
>   Xen-specific caveats apply.
>
> As per my original email, I'm not against the renaming of the stub
> parameters if we standardise the rest of the details, but I believe
> that's orthogonal to the Xen Dom0 case.
>

Xen will not boot the kernel via the stub, but directly. It needs to
supply a EFI like environment so that the kernel can boot via ACPI.
There is no reason whatsoever to mock up boot services or other pieces
of UEFI functionality that are not needed. The core kernel does not
call any boot services or SetVirtualAddressMap/ConvertPointer, and
there is already paravirtualized plumbing in place for the remaining
runtime services.

Hence my claim earlier that we should cope with the runtime services
pointer being NULL, since that is really the only thing standing in
the way from the kernel side. If you feel that violates the spec in
some way, we could at least conditionalise it on EFI_RUNTIME_SERVICES
having been set already, this gives the Xen code a chance of
overriding them.
Ian Campbell Sept. 14, 2015, 9:57 a.m. UTC | #26
On Mon, 2015-09-14 at 11:43 +0200, Ard Biesheuvel wrote:

> Xen will not boot the kernel via the stub, but directly. It needs to
> supply a EFI like environment so that the kernel can boot via ACPI.
> There is no reason whatsoever to mock up boot services or other pieces
> of UEFI functionality that are not needed.

I'm correct that on native the EFI stub calls ExitBootServices, right?

So the flow for native is:

EFI -> Linux EFI Stub -> Exit Boot Services -> Non-EFI Linux head.S entrypoint

For Xen it is more like:

Xen domain builder -------  ...    ... ------> Non-EFI Linux head.S entrypoint

>  The core kernel does not call any boot services

And it cannot because ExitBootServices has already been called.

I think given all that there should no reason at all for Xen to be
providing boot services.

>  or SetVirtualAddressMap/ConvertPointer, and

These two are RTS, so in principal it could.

(I'm not sure about ConvertPointer, is it useful for OS kernels, or just
for "UEFI components" mentioned at http://wiki.phoenix.com/wiki/index.php/E
FI_RUNTIME_SERVICES#ConvertPointer.28.29 ?)

> there is already paravirtualized plumbing in place for the remaining
> runtime services.
> 
> Hence my claim earlier that we should cope with the runtime services
> pointer being NULL, since that is really the only thing standing in
> the way from the kernel side. If you feel that violates the spec in
> some way, we could at least conditionalise it on EFI_RUNTIME_SERVICES
> having been set already, this gives the Xen code a chance of
> overriding them.
>
Ard Biesheuvel Sept. 14, 2015, 10:02 a.m. UTC | #27
On 14 September 2015 at 11:57, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Mon, 2015-09-14 at 11:43 +0200, Ard Biesheuvel wrote:
>
>> Xen will not boot the kernel via the stub, but directly. It needs to
>> supply a EFI like environment so that the kernel can boot via ACPI.
>> There is no reason whatsoever to mock up boot services or other pieces
>> of UEFI functionality that are not needed.
>
> I'm correct that on native the EFI stub calls ExitBootServices, right?
>
> So the flow for native is:
>
> EFI -> Linux EFI Stub -> Exit Boot Services -> Non-EFI Linux head.S entrypoint
>
> For Xen it is more like:
>
> Xen domain builder -------  ...    ... ------> Non-EFI Linux head.S entrypoint
>
>>  The core kernel does not call any boot services
>
> And it cannot because ExitBootServices has already been called.
>
> I think given all that there should no reason at all for Xen to be
> providing boot services.
>

Indeed.

>>  or SetVirtualAddressMap/ConvertPointer, and
>
> These two are RTS, so in principal it could.
>
> (I'm not sure about ConvertPointer, is it useful for OS kernels, or just
> for "UEFI components" mentioned at http://wiki.phoenix.com/wiki/index.php/E
> FI_RUNTIME_SERVICES#ConvertPointer.28.29 ?)
>

No, there is no point. The stub calls SetVirtualAddressMap, so the
kernel proper can never call it, since it can only be called once.
ConvertPointer has little utility outside of the UEFI runtime
components that are invoked during SetVirtualAddressMap, so I don't
see a reason to supply that either.
Ian Campbell Sept. 14, 2015, 10:25 a.m. UTC | #28
On Mon, 2015-09-14 at 12:02 +0200, Ard Biesheuvel wrote:
> On 14 September 2015 at 11:57, Ian Campbell <ian.campbell@citrix.com> wrote:
> > >  or SetVirtualAddressMap/ConvertPointer, and
> > 
> > These two are RTS, so in principal it could.
> > 
> > (I'm not sure about ConvertPointer, is it useful for OS kernels, or
> > just
> > for "UEFI components" mentioned at 
> > http://wiki.phoenix.com/wiki/index.php/E
> > FI_RUNTIME_SERVICES#ConvertPointer.28.29 ?)
> > 
> 
> No, there is no point. The stub calls SetVirtualAddressMap, so the
> kernel proper can never call it, since it can only be called once.

I see. And changing this such that it was delayed until the kernel proper
would be a _major_ shift in the policy of separation between the UEFI stub
and the kernel proper. 

> ConvertPointer has little utility outside of the UEFI runtime
> components that are invoked during SetVirtualAddressMap,

That's the impression I was getting too. Thanks for confirming.

>  so I don't see a reason to supply that either.

Ack.

Ian.
Jan Beulich Sept. 14, 2015, 10:39 a.m. UTC | #29
>>> On 14.09.15 at 11:36, <ard.biesheuvel@linaro.org> wrote:
> On 14 September 2015 at 11:31, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>> My understanding is that if there are no EFI_MEMORY_RUNTIME regions, it
>> means we can't use runtime services and should not set the bit
>> EFI_RUNTIME_SERVICES of efi.flags. But if efi_virtmap_init() return
>> true, the bit will be set.
>>
> 
> As I said, if you don't want the EFI_RUNTIME_SERVICES bit to be set
> for other reasons, don't rig efi_virtmap_init() to return false when
> it shouldn't.
> 
>>> The absence of such regions is allowed by the spec, so
>>> efi_virtmap_init() is correct imo to return success.
>>>
>> Sorry, not well know about the spec. Could you point out where the spec
>> says this?
>>
> 
> Well, I think it doesn't work that way. You are claiming that a memory
> map without at least one EFI_MEMORY_RUNTIME constitutes an error
> condition, so the burden is on you to provide a reference to the spec
> that says you must have at least one such region.

Sure, from a spec pov you're right. But where would runtime
services code/data live when there's not a single region marked
as needing a runtime mapping. IOW while the spec doesn't say
so, assuming no runtime services when there's not at least one
executable region with the runtime flag set could serve as a stop
gap measure against flawed firmware.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Ard Biesheuvel Sept. 14, 2015, 11:16 a.m. UTC | #30
On 14 September 2015 at 12:39, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 14.09.15 at 11:36, <ard.biesheuvel@linaro.org> wrote:
>> On 14 September 2015 at 11:31, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
>>> My understanding is that if there are no EFI_MEMORY_RUNTIME regions, it
>>> means we can't use runtime services and should not set the bit
>>> EFI_RUNTIME_SERVICES of efi.flags. But if efi_virtmap_init() return
>>> true, the bit will be set.
>>>
>>
>> As I said, if you don't want the EFI_RUNTIME_SERVICES bit to be set
>> for other reasons, don't rig efi_virtmap_init() to return false when
>> it shouldn't.
>>
>>>> The absence of such regions is allowed by the spec, so
>>>> efi_virtmap_init() is correct imo to return success.
>>>>
>>> Sorry, not well know about the spec. Could you point out where the spec
>>> says this?
>>>
>>
>> Well, I think it doesn't work that way. You are claiming that a memory
>> map without at least one EFI_MEMORY_RUNTIME constitutes an error
>> condition, so the burden is on you to provide a reference to the spec
>> that says you must have at least one such region.
>
> Sure, from a spec pov you're right. But where would runtime
> services code/data live when there's not a single region marked
> as needing a runtime mapping. IOW while the spec doesn't say
> so, assuming no runtime services when there's not at least one
> executable region with the runtime flag set could serve as a stop
> gap measure against flawed firmware.
>

That in itself is a fair point. But the argument being made was that
it is in itself a bug for no EFI_MEMORY_RUNTIME regions to exist,
while the actual purpose of the patch is to prevent the
RuntimeServices pointer from being dereferenced on platforms like Xen
that may not be able to implement them.

But actually, even in case of Xen, you will need some
EFI_MEMORY_RUNTIME regions anyway, since the f/w vendor field and the
config and runtime services table pointers are translated to virtual
addresses by the firmware, which means the OS needs to translate them
back to physical addresses in order to dereference them before the VA
mapping is up. (I still think not using SetVirtualAddressMap() at all
would be the best approach for arm64, but unfortunately, most of my
colleagues disagree with me)
Jan Beulich Sept. 14, 2015, 11:34 a.m. UTC | #31
>>> On 14.09.15 at 13:16, <ard.biesheuvel@linaro.org> wrote:
> (I still think not using SetVirtualAddressMap() at all
> would be the best approach for arm64, but unfortunately, most of my
> colleagues disagree with me)

Any reasons they have? I'm curious because we run x86 Xen without
using this function ...

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Daniel Kiper Sept. 14, 2015, 12:28 p.m. UTC | #32
On Mon, Sep 14, 2015 at 11:43:27AM +0200, Ard Biesheuvel wrote:
> On 14 September 2015 at 11:25, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Sat, Sep 12, 2015 at 12:36:55PM +0100, Daniel Kiper wrote:
> >> On Fri, Sep 11, 2015 at 05:25:59PM +0100, Mark Rutland wrote:
> >> > On Fri, Sep 11, 2015 at 01:46:43PM +0100, Daniel Kiper wrote:
> >> > > On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote:
> >>
> >> [...]
> >>
> >> > > > What's troublesome with the boot services?
> >> > > >
> >> > > > What can't be simulated?
> >> > >
> >> > > How do you want to access bare metal EFI boot services from dom0 if they
> >> > > were shutdown long time ago before loading dom0 image?
> >> >
> >> > I don't want to.
> >> >
> >> > I asked "What can't be simulated?" because I assumed everything
> >> > necessary/mandatory could be simulated without needinng access to any
> >> > real EFI boot services.
> >> >
> >> > As far as I can see all that's necessary is to provide a compatible
> >> > interface.
> >>
> >> Could you be more precise what do you need? Please enumerate. UEFI spec has
> >> more than 2500 pages and I do not think that we need all stuff in dom0.
> >>
> >> > > What do you need from EFI boot services in dom0?
> >> >
> >> > The ability to call ExitBootServices() and SetVirtualAddressMap() on a
> >> > _virtual_ address map for _virtual_ services provided by the hypervisor.
> >>
> >> I am confused. Why do you need that? Please remember, EFI is owned and
> >> operated by Xen hypervisor. dom0 does not have direct access to EFI.
> >
> > Let's take a step back.
> >
> > My objection here is to passing the Dom0 kernel properties as if it were
> > booted with direct access to a full UEFI, then later fixing that up
> > (when Xen is detected and we apply its hypercall EFI implementation).
> >
>
> To be honest, I don't think that has ever been suggested here. What
> was suggested is to provide a minimal EFI like environment that allows
> the post-stub EFI code to proceed and find the ACPI root pointer.
>
> > If the kernel cannot use EFI natively, why pretend to the kernel that it
> > can? The hypercall implementation is _not_ EFI (though it provides
> > access to some services).
> >
>
> To get access to the ACPI root pointer, for which there is only one
> specified way of obtaining it on ARM, which is via the UEFI
> configuration table database
>
> > The two ways I can see providing Dom0 with EFI services are:
> >
> > * Have Xen create shims for any services, in which any hypercalls live,
> >   and pass these to the kernel with a virtual system table. This keeps
> >   the interface to the kernel the same regardless of Xen.
> >
> > * Have the kernel detect Xen EFI capability via Xen, without passing the
> >   usual native EFI parameters. This can then be installed into the
> >   kernel in a Xen-specific manner, and we know from the outset that
> >   Xen-specific caveats apply.
> >
> > As per my original email, I'm not against the renaming of the stub
> > parameters if we standardise the rest of the details, but I believe
> > that's orthogonal to the Xen Dom0 case.
> >
>
> Xen will not boot the kernel via the stub, but directly. It needs to
> supply a EFI like environment so that the kernel can boot via ACPI.
> There is no reason whatsoever to mock up boot services or other pieces
> of UEFI functionality that are not needed. The core kernel does not
> call any boot services or SetVirtualAddressMap/ConvertPointer, and
> there is already paravirtualized plumbing in place for the remaining
> runtime services.
>
> Hence my claim earlier that we should cope with the runtime services
> pointer being NULL, since that is really the only thing standing in

I suppose that you thought about EFI_INVALID_TABLE_ADDR...

> the way from the kernel side. If you feel that violates the spec in

If yes then you should know that dom0 on x86 EFI platform works
with efi.runtime == EFI_INVALID_TABLE_ADDR without any issue.
So, I think that all problems are solved.

Daniel
Ard Biesheuvel Sept. 14, 2015, 1:09 p.m. UTC | #33
On 14 September 2015 at 14:28, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> On Mon, Sep 14, 2015 at 11:43:27AM +0200, Ard Biesheuvel wrote:
>> On 14 September 2015 at 11:25, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Sat, Sep 12, 2015 at 12:36:55PM +0100, Daniel Kiper wrote:
>> >> On Fri, Sep 11, 2015 at 05:25:59PM +0100, Mark Rutland wrote:
>> >> > On Fri, Sep 11, 2015 at 01:46:43PM +0100, Daniel Kiper wrote:
>> >> > > On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote:
>> >>
>> >> [...]
>> >>
>> >> > > > What's troublesome with the boot services?
>> >> > > >
>> >> > > > What can't be simulated?
>> >> > >
>> >> > > How do you want to access bare metal EFI boot services from dom0 if they
>> >> > > were shutdown long time ago before loading dom0 image?
>> >> >
>> >> > I don't want to.
>> >> >
>> >> > I asked "What can't be simulated?" because I assumed everything
>> >> > necessary/mandatory could be simulated without needinng access to any
>> >> > real EFI boot services.
>> >> >
>> >> > As far as I can see all that's necessary is to provide a compatible
>> >> > interface.
>> >>
>> >> Could you be more precise what do you need? Please enumerate. UEFI spec has
>> >> more than 2500 pages and I do not think that we need all stuff in dom0.
>> >>
>> >> > > What do you need from EFI boot services in dom0?
>> >> >
>> >> > The ability to call ExitBootServices() and SetVirtualAddressMap() on a
>> >> > _virtual_ address map for _virtual_ services provided by the hypervisor.
>> >>
>> >> I am confused. Why do you need that? Please remember, EFI is owned and
>> >> operated by Xen hypervisor. dom0 does not have direct access to EFI.
>> >
>> > Let's take a step back.
>> >
>> > My objection here is to passing the Dom0 kernel properties as if it were
>> > booted with direct access to a full UEFI, then later fixing that up
>> > (when Xen is detected and we apply its hypercall EFI implementation).
>> >
>>
>> To be honest, I don't think that has ever been suggested here. What
>> was suggested is to provide a minimal EFI like environment that allows
>> the post-stub EFI code to proceed and find the ACPI root pointer.
>>
>> > If the kernel cannot use EFI natively, why pretend to the kernel that it
>> > can? The hypercall implementation is _not_ EFI (though it provides
>> > access to some services).
>> >
>>
>> To get access to the ACPI root pointer, for which there is only one
>> specified way of obtaining it on ARM, which is via the UEFI
>> configuration table database
>>
>> > The two ways I can see providing Dom0 with EFI services are:
>> >
>> > * Have Xen create shims for any services, in which any hypercalls live,
>> >   and pass these to the kernel with a virtual system table. This keeps
>> >   the interface to the kernel the same regardless of Xen.
>> >
>> > * Have the kernel detect Xen EFI capability via Xen, without passing the
>> >   usual native EFI parameters. This can then be installed into the
>> >   kernel in a Xen-specific manner, and we know from the outset that
>> >   Xen-specific caveats apply.
>> >
>> > As per my original email, I'm not against the renaming of the stub
>> > parameters if we standardise the rest of the details, but I believe
>> > that's orthogonal to the Xen Dom0 case.
>> >
>>
>> Xen will not boot the kernel via the stub, but directly. It needs to
>> supply a EFI like environment so that the kernel can boot via ACPI.
>> There is no reason whatsoever to mock up boot services or other pieces
>> of UEFI functionality that are not needed. The core kernel does not
>> call any boot services or SetVirtualAddressMap/ConvertPointer, and
>> there is already paravirtualized plumbing in place for the remaining
>> runtime services.
>>
>> Hence my claim earlier that we should cope with the runtime services
>> pointer being NULL, since that is really the only thing standing in
>
> I suppose that you thought about EFI_INVALID_TABLE_ADDR...
>

Simply whatever we decide, so perhaps EFI_INVALID_TABLE_ADDR is better
if x86 uses that already. But that value is still outside of the UEFI
spec, so in that sense, it is not more appropriate than NULL.

>> the way from the kernel side. If you feel that violates the spec in
>
> If yes then you should know that dom0 on x86 EFI platform works
> with efi.runtime == EFI_INVALID_TABLE_ADDR without any issue.
> So, I think that all problems are solved.
>

So there is precedent, which is good. But please note that x86 has a
lot of baggage and *lots* of quirks for buggy firmware that was only
ever tested with Windows. So before blindly copying x86 when it comes
to UEFI interworking, we still need to have the discussion whether
what x86 is appropriate for ARM as well (since it does not have the
above problems)
Daniel Kiper Sept. 14, 2015, 1:57 p.m. UTC | #34
On Mon, Sep 14, 2015 at 03:09:34PM +0200, Ard Biesheuvel wrote:
> On 14 September 2015 at 14:28, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > On Mon, Sep 14, 2015 at 11:43:27AM +0200, Ard Biesheuvel wrote:
> >> On 14 September 2015 at 11:25, Mark Rutland <mark.rutland@arm.com> wrote:
> >> > On Sat, Sep 12, 2015 at 12:36:55PM +0100, Daniel Kiper wrote:
> >> >> On Fri, Sep 11, 2015 at 05:25:59PM +0100, Mark Rutland wrote:
> >> >> > On Fri, Sep 11, 2015 at 01:46:43PM +0100, Daniel Kiper wrote:
> >> >> > > On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote:
> >> >>
> >> >> [...]
> >> >>
> >> >> > > > What's troublesome with the boot services?
> >> >> > > >
> >> >> > > > What can't be simulated?
> >> >> > >
> >> >> > > How do you want to access bare metal EFI boot services from dom0 if they
> >> >> > > were shutdown long time ago before loading dom0 image?
> >> >> >
> >> >> > I don't want to.
> >> >> >
> >> >> > I asked "What can't be simulated?" because I assumed everything
> >> >> > necessary/mandatory could be simulated without needinng access to any
> >> >> > real EFI boot services.
> >> >> >
> >> >> > As far as I can see all that's necessary is to provide a compatible
> >> >> > interface.
> >> >>
> >> >> Could you be more precise what do you need? Please enumerate. UEFI spec has
> >> >> more than 2500 pages and I do not think that we need all stuff in dom0.
> >> >>
> >> >> > > What do you need from EFI boot services in dom0?
> >> >> >
> >> >> > The ability to call ExitBootServices() and SetVirtualAddressMap() on a
> >> >> > _virtual_ address map for _virtual_ services provided by the hypervisor.
> >> >>
> >> >> I am confused. Why do you need that? Please remember, EFI is owned and
> >> >> operated by Xen hypervisor. dom0 does not have direct access to EFI.
> >> >
> >> > Let's take a step back.
> >> >
> >> > My objection here is to passing the Dom0 kernel properties as if it were
> >> > booted with direct access to a full UEFI, then later fixing that up
> >> > (when Xen is detected and we apply its hypercall EFI implementation).
> >> >
> >>
> >> To be honest, I don't think that has ever been suggested here. What
> >> was suggested is to provide a minimal EFI like environment that allows
> >> the post-stub EFI code to proceed and find the ACPI root pointer.
> >>
> >> > If the kernel cannot use EFI natively, why pretend to the kernel that it
> >> > can? The hypercall implementation is _not_ EFI (though it provides
> >> > access to some services).
> >> >
> >>
> >> To get access to the ACPI root pointer, for which there is only one
> >> specified way of obtaining it on ARM, which is via the UEFI
> >> configuration table database
> >>
> >> > The two ways I can see providing Dom0 with EFI services are:
> >> >
> >> > * Have Xen create shims for any services, in which any hypercalls live,
> >> >   and pass these to the kernel with a virtual system table. This keeps
> >> >   the interface to the kernel the same regardless of Xen.
> >> >
> >> > * Have the kernel detect Xen EFI capability via Xen, without passing the
> >> >   usual native EFI parameters. This can then be installed into the
> >> >   kernel in a Xen-specific manner, and we know from the outset that
> >> >   Xen-specific caveats apply.
> >> >
> >> > As per my original email, I'm not against the renaming of the stub
> >> > parameters if we standardise the rest of the details, but I believe
> >> > that's orthogonal to the Xen Dom0 case.
> >> >
> >>
> >> Xen will not boot the kernel via the stub, but directly. It needs to
> >> supply a EFI like environment so that the kernel can boot via ACPI.
> >> There is no reason whatsoever to mock up boot services or other pieces
> >> of UEFI functionality that are not needed. The core kernel does not
> >> call any boot services or SetVirtualAddressMap/ConvertPointer, and
> >> there is already paravirtualized plumbing in place for the remaining
> >> runtime services.
> >>
> >> Hence my claim earlier that we should cope with the runtime services
> >> pointer being NULL, since that is really the only thing standing in
> >
> > I suppose that you thought about EFI_INVALID_TABLE_ADDR...
> >
>
> Simply whatever we decide, so perhaps EFI_INVALID_TABLE_ADDR is better
> if x86 uses that already. But that value is still outside of the UEFI
> spec, so in that sense, it is not more appropriate than NULL.

Yep, you are right. However, I hope (I am not sure) that it was good reason
behind choosing that value in Linux kernel and I think that in EFI related
code it should be used as it is used right now.

> >> the way from the kernel side. If you feel that violates the spec in
> >
> > If yes then you should know that dom0 on x86 EFI platform works
> > with efi.runtime == EFI_INVALID_TABLE_ADDR without any issue.
> > So, I think that all problems are solved.
> >
>
> So there is precedent, which is good. But please note that x86 has a
> lot of baggage and *lots* of quirks for buggy firmware that was only
> ever tested with Windows. So before blindly copying x86 when it comes
> to UEFI interworking, we still need to have the discussion whether
> what x86 is appropriate for ARM as well (since it does not have the
> above problems)

EFI was designed as much as possible platform independent stuff and most
of Linux kernel EFI code is platform independent (including Xen EFI related
stuff). Additionally, as I can see currently existing implementation can run
at least on IA64 and ARM64 (OK, Xen stuff does not work there) and for sure
they do not care about x86 quirks. So, I think this is good starting point
for dom0 implementation on ARM EFI. However, I am not trying to say that it
will work for sure. Probably something should be changed but I think it should
not be a big deal (well, I hope, taking into account this thread and what I know).

Daniel
Shannon Zhao Sept. 17, 2015, 11:43 a.m. UTC | #35
Hi,

From the comments on this patch, IIUC, we don't object to the change
brought by this patch. What we didn't reach an agreement is how to
support runtime service for Dom0. Right? If so, I think this patch
doesn't conflict with adding support for runtime service in the future.
So could we move this patch forward? Then I could continue working on
adding ARM ACPI support on Xen.

Any comments?

Thanks,
diff mbox

Patch

diff --git a/Documentation/arm/uefi.txt b/Documentation/arm/uefi.txt
index d60030a..8c83243 100644
--- a/Documentation/arm/uefi.txt
+++ b/Documentation/arm/uefi.txt
@@ -45,18 +45,18 @@  following parameters:
 ________________________________________________________________________________
 Name                      | Size   | Description
 ================================================================================
-linux,uefi-system-table   | 64-bit | Physical address of the UEFI System Table.
+uefi-system-table         | 64-bit | Physical address of the UEFI System Table.
 --------------------------------------------------------------------------------
-linux,uefi-mmap-start     | 64-bit | Physical address of the UEFI memory map,
+uefi-mmap-start           | 64-bit | Physical address of the UEFI memory map,
                           |        | populated by the UEFI GetMemoryMap() call.
 --------------------------------------------------------------------------------
-linux,uefi-mmap-size      | 32-bit | Size in bytes of the UEFI memory map
+uefi-mmap-size            | 32-bit | Size in bytes of the UEFI memory map
                           |        | pointed to in previous entry.
 --------------------------------------------------------------------------------
-linux,uefi-mmap-desc-size | 32-bit | Size in bytes of each entry in the UEFI
+uefi-mmap-desc-size       | 32-bit | Size in bytes of each entry in the UEFI
                           |        | memory map.
 --------------------------------------------------------------------------------
-linux,uefi-mmap-desc-ver  | 32-bit | Version of the mmap descriptor format.
+uefi-mmap-desc-ver        | 32-bit | Version of the mmap descriptor format.
 --------------------------------------------------------------------------------
 linux,uefi-stub-kern-ver  | string | Copy of linux_banner from build.
 --------------------------------------------------------------------------------
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index d6144e3..3878715 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -481,11 +481,11 @@  static __initdata struct {
 	int offset;
 	int size;
 } dt_params[] = {
-	UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
-	UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
-	UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
-	UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
-	UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
+	UEFI_PARAM("System Table", "uefi-system-table", system_table),
+	UEFI_PARAM("MemMap Address", "uefi-mmap-start", mmap),
+	UEFI_PARAM("MemMap Size", "uefi-mmap-size", mmap_size),
+	UEFI_PARAM("MemMap Desc. Size", "uefi-mmap-desc-size", desc_size),
+	UEFI_PARAM("MemMap Desc. Version", "uefi-mmap-desc-ver", desc_ver)
 };
 
 struct param_info {
diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index ef5d764..e94589a 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -118,31 +118,31 @@  efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
 	/* Add FDT entries for EFI runtime services in chosen node. */
 	node = fdt_subnode_offset(fdt, 0, "chosen");
 	fdt_val64 = cpu_to_fdt64((u64)(unsigned long)sys_table);
-	status = fdt_setprop(fdt, node, "linux,uefi-system-table",
+	status = fdt_setprop(fdt, node, "uefi-system-table",
 			     &fdt_val64, sizeof(fdt_val64));
 	if (status)
 		goto fdt_set_fail;
 
 	fdt_val64 = cpu_to_fdt64((u64)(unsigned long)memory_map);
-	status = fdt_setprop(fdt, node, "linux,uefi-mmap-start",
+	status = fdt_setprop(fdt, node, "uefi-mmap-start",
 			     &fdt_val64,  sizeof(fdt_val64));
 	if (status)
 		goto fdt_set_fail;
 
 	fdt_val32 = cpu_to_fdt32(map_size);
-	status = fdt_setprop(fdt, node, "linux,uefi-mmap-size",
+	status = fdt_setprop(fdt, node, "uefi-mmap-size",
 			     &fdt_val32,  sizeof(fdt_val32));
 	if (status)
 		goto fdt_set_fail;
 
 	fdt_val32 = cpu_to_fdt32(desc_size);
-	status = fdt_setprop(fdt, node, "linux,uefi-mmap-desc-size",
+	status = fdt_setprop(fdt, node, "uefi-mmap-desc-size",
 			     &fdt_val32, sizeof(fdt_val32));
 	if (status)
 		goto fdt_set_fail;
 
 	fdt_val32 = cpu_to_fdt32(desc_ver);
-	status = fdt_setprop(fdt, node, "linux,uefi-mmap-desc-ver",
+	status = fdt_setprop(fdt, node, "uefi-mmap-desc-ver",
 			     &fdt_val32, sizeof(fdt_val32));
 	if (status)
 		goto fdt_set_fail;