diff mbox

[Xen-devel,FOR-4.5] xen: arm: Do not enable EFI in dom0 since it is not yet supported.

Message ID 1413217030-9170-1-git-send-email-suravee.suthikulpanit@amd.com
State New
Headers show

Commit Message

Suthikulpanit, Suravee Oct. 13, 2014, 4:17 p.m. UTC
From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Since EFI is not yet supported in dom0, we need to remove the following
properties from the chosen node:

  * linux,uefi-mmap-start
  * linux,uefi-mmap-size
  * linux,uefi-mmap-desc-size
  * linux,uefi-mmap-desc-ver

These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
and used by dom0 kernel to enable EFI.

Cc: Julien Grall <julien.grall@linaro.org>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Roy Franz <roy.franz@linaro.org>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 xen/arch/arm/domain_build.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Julien Grall Oct. 13, 2014, 4:20 p.m. UTC | #1
(CC vijay)

Hi Suravee,

On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> Since EFI is not yet supported in dom0, we need to remove the following
> properties from the chosen node:
> 
>   * linux,uefi-mmap-start
>   * linux,uefi-mmap-size
>   * linux,uefi-mmap-desc-size
>   * linux,uefi-mmap-desc-ver
> 
> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
> and used by dom0 kernel to enable EFI.
> 
> Cc: Julien Grall <julien.grall@linaro.org>
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Roy Franz <roy.franz@linaro.org>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

> ---
>  xen/arch/arm/domain_build.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 90abc3a..f1d0d20 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -423,12 +423,21 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
>           *   bootargs (from module #1, above).
>           * * remove bootargs,  xen,dom0-bootargs, xen,xen-bootargs,
>           *   linux,initrd-start and linux,initrd-end.
> +         * * remove bootargs, linux,uefi-system-table,
> +         *   linux,uefi-mmap-start, linux,uefi-mmap-size,
> +         *   linux,uefi-mmap-desc-size, and linux,uefi-mmap-desc-ver
> +         *   (since EFI boot is not currently supported in dom0).
>           */
>          if ( dt_node_path_is_equal(node, "/chosen") )
>          {
>              if ( dt_property_name_is_equal(prop, "xen,xen-bootargs") ||
>                   dt_property_name_is_equal(prop, "linux,initrd-start") ||
> -                 dt_property_name_is_equal(prop, "linux,initrd-end") )
> +                 dt_property_name_is_equal(prop, "linux,initrd-end") ||
> +                 dt_property_name_is_equal(prop, "linux,uefi-system-table") ||
> +                 dt_property_name_is_equal(prop, "linux,uefi-mmap-start") ||
> +                 dt_property_name_is_equal(prop, "linux,uefi-mmap-size") ||
> +                 dt_property_name_is_equal(prop, "linux,uefi-mmap-desc-size") ||
> +                 dt_property_name_is_equal(prop, "linux,uefi-mmap-desc-ver"))
>                  continue;
>  
>              if ( dt_property_name_is_equal(prop, "xen,dom0-bootargs") )
>
Roy Franz Oct. 13, 2014, 7:37 p.m. UTC | #2
On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
> (CC vijay)
>
> Hi Suravee,
>
> On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>
>> Since EFI is not yet supported in dom0, we need to remove the following
>> properties from the chosen node:
>>
>>   * linux,uefi-mmap-start
>>   * linux,uefi-mmap-size
>>   * linux,uefi-mmap-desc-size
>>   * linux,uefi-mmap-desc-ver
>>
>> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
>> and used by dom0 kernel to enable EFI.
>>
>> Cc: Julien Grall <julien.grall@linaro.org>
>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Cc: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Roy Franz <roy.franz@linaro.org>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>
> Reviewed-by: Julien Grall <julien.grall@linaro.org>

Reviewed-by: Roy Franz <roy.franz@linaro.org>
>
> Regards,
>
>> ---
>>  xen/arch/arm/domain_build.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 90abc3a..f1d0d20 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -423,12 +423,21 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,
>>           *   bootargs (from module #1, above).
>>           * * remove bootargs,  xen,dom0-bootargs, xen,xen-bootargs,
>>           *   linux,initrd-start and linux,initrd-end.
>> +         * * remove bootargs, linux,uefi-system-table,
>> +         *   linux,uefi-mmap-start, linux,uefi-mmap-size,
>> +         *   linux,uefi-mmap-desc-size, and linux,uefi-mmap-desc-ver
>> +         *   (since EFI boot is not currently supported in dom0).
>>           */
>>          if ( dt_node_path_is_equal(node, "/chosen") )
>>          {
>>              if ( dt_property_name_is_equal(prop, "xen,xen-bootargs") ||
>>                   dt_property_name_is_equal(prop, "linux,initrd-start") ||
>> -                 dt_property_name_is_equal(prop, "linux,initrd-end") )
>> +                 dt_property_name_is_equal(prop, "linux,initrd-end") ||
>> +                 dt_property_name_is_equal(prop, "linux,uefi-system-table") ||
>> +                 dt_property_name_is_equal(prop, "linux,uefi-mmap-start") ||
>> +                 dt_property_name_is_equal(prop, "linux,uefi-mmap-size") ||
>> +                 dt_property_name_is_equal(prop, "linux,uefi-mmap-desc-size") ||
>> +                 dt_property_name_is_equal(prop, "linux,uefi-mmap-desc-ver"))
>>                  continue;
>>
>>              if ( dt_property_name_is_equal(prop, "xen,dom0-bootargs") )
>>
>
> --
> Julien Grall
Ian Campbell Oct. 14, 2014, 7:21 a.m. UTC | #3
On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
> On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
> > (CC vijay)
> >
> > Hi Suravee,
> >
> > On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
> >> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>
> >> Since EFI is not yet supported in dom0, we need to remove the following
> >> properties from the chosen node:
> >>
> >>   * linux,uefi-mmap-start
> >>   * linux,uefi-mmap-size
> >>   * linux,uefi-mmap-desc-size
> >>   * linux,uefi-mmap-desc-ver
> >>
> >> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
> >> and used by dom0 kernel to enable EFI.
> >>
> >> Cc: Julien Grall <julien.grall@linaro.org>
> >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >> Cc: Ian Campbell <ian.campbell@citrix.com>
> >> Cc: Roy Franz <roy.franz@linaro.org>
> >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >
> > Reviewed-by: Julien Grall <julien.grall@linaro.org>
> 
> Reviewed-by: Roy Franz <roy.franz@linaro.org>

Acked-by: Ian Campbell <ian.campbell@.citrix.com>

But I do wonder why we don't just drop the hos /chosen node -- I think
everything which should be in there is fabricated by the h/visor anyway.

Ian.
Ian Campbell Oct. 14, 2014, 9:59 a.m. UTC | #4
On Tue, 2014-10-14 at 08:21 +0100, Ian Campbell wrote:
> On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
> > On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
> > > (CC vijay)
> > >
> > > Hi Suravee,
> > >
> > > On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
> > >> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> > >>
> > >> Since EFI is not yet supported in dom0, we need to remove the following
> > >> properties from the chosen node:
> > >>
> > >>   * linux,uefi-mmap-start
> > >>   * linux,uefi-mmap-size
> > >>   * linux,uefi-mmap-desc-size
> > >>   * linux,uefi-mmap-desc-ver
> > >>
> > >> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
> > >> and used by dom0 kernel to enable EFI.
> > >>
> > >> Cc: Julien Grall <julien.grall@linaro.org>
> > >> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > >> Cc: Ian Campbell <ian.campbell@citrix.com>
> > >> Cc: Roy Franz <roy.franz@linaro.org>
> > >> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> > >
> > > Reviewed-by: Julien Grall <julien.grall@linaro.org>
> > 
> > Reviewed-by: Roy Franz <roy.franz@linaro.org>
> 
> Acked-by: Ian Campbell <ian.campbell@.citrix.com>

Applied.
Julien Grall Oct. 14, 2014, 12:28 p.m. UTC | #5
Hi Ian,

On 10/14/2014 08:21 AM, Ian Campbell wrote:
> On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
>> On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
>>> (CC vijay)
>>>
>>> Hi Suravee,
>>>
>>> On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
>>>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>>>
>>>> Since EFI is not yet supported in dom0, we need to remove the following
>>>> properties from the chosen node:
>>>>
>>>>   * linux,uefi-mmap-start
>>>>   * linux,uefi-mmap-size
>>>>   * linux,uefi-mmap-desc-size
>>>>   * linux,uefi-mmap-desc-ver
>>>>
>>>> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
>>>> and used by dom0 kernel to enable EFI.
>>>>
>>>> Cc: Julien Grall <julien.grall@linaro.org>
>>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>> Cc: Ian Campbell <ian.campbell@citrix.com>
>>>> Cc: Roy Franz <roy.franz@linaro.org>
>>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>>
>>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
>>
>> Reviewed-by: Roy Franz <roy.franz@linaro.org>
> 
> Acked-by: Ian Campbell <ian.campbell@.citrix.com>
> 
> But I do wonder why we don't just drop the hos /chosen node -- I think
> everything which should be in there is fabricated by the h/visor anyway.

I though about the same things yesterday, but the /chosen node may
contain other properties (such as OS specific) that we may need to pass
to DOM0.

So a blacklist looks the better solution for now.

Regards,
Ian Campbell Oct. 14, 2014, 12:49 p.m. UTC | #6
On Tue, 2014-10-14 at 13:28 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 10/14/2014 08:21 AM, Ian Campbell wrote:
> > On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
> >> On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
> >>> (CC vijay)
> >>>
> >>> Hi Suravee,
> >>>
> >>> On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
> >>>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>>>
> >>>> Since EFI is not yet supported in dom0, we need to remove the following
> >>>> properties from the chosen node:
> >>>>
> >>>>   * linux,uefi-mmap-start
> >>>>   * linux,uefi-mmap-size
> >>>>   * linux,uefi-mmap-desc-size
> >>>>   * linux,uefi-mmap-desc-ver
> >>>>
> >>>> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
> >>>> and used by dom0 kernel to enable EFI.
> >>>>
> >>>> Cc: Julien Grall <julien.grall@linaro.org>
> >>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>>> Cc: Ian Campbell <ian.campbell@citrix.com>
> >>>> Cc: Roy Franz <roy.franz@linaro.org>
> >>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >>>
> >>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> >>
> >> Reviewed-by: Roy Franz <roy.franz@linaro.org>
> > 
> > Acked-by: Ian Campbell <ian.campbell@.citrix.com>
> > 
> > But I do wonder why we don't just drop the hos /chosen node -- I think
> > everything which should be in there is fabricated by the h/visor anyway.
> 
> I though about the same things yesterday, but the /chosen node may
> contain other properties (such as OS specific) that we may need to pass
> to DOM0.

What sort of thing are you thinking of here?

> 
> So a blacklist looks the better solution for now.
> 
> Regards,
>
Julien Grall Oct. 14, 2014, 12:58 p.m. UTC | #7
On 10/14/2014 01:49 PM, Ian Campbell wrote:
> On Tue, 2014-10-14 at 13:28 +0100, Julien Grall wrote:
>> Hi Ian,
>>
>> On 10/14/2014 08:21 AM, Ian Campbell wrote:
>>> On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
>>>> On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>> (CC vijay)
>>>>>
>>>>> Hi Suravee,
>>>>>
>>>>> On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
>>>>>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>>>>>
>>>>>> Since EFI is not yet supported in dom0, we need to remove the following
>>>>>> properties from the chosen node:
>>>>>>
>>>>>>   * linux,uefi-mmap-start
>>>>>>   * linux,uefi-mmap-size
>>>>>>   * linux,uefi-mmap-desc-size
>>>>>>   * linux,uefi-mmap-desc-ver
>>>>>>
>>>>>> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
>>>>>> and used by dom0 kernel to enable EFI.
>>>>>>
>>>>>> Cc: Julien Grall <julien.grall@linaro.org>
>>>>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>>>> Cc: Ian Campbell <ian.campbell@citrix.com>
>>>>>> Cc: Roy Franz <roy.franz@linaro.org>
>>>>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>>>>
>>>>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
>>>>
>>>> Reviewed-by: Roy Franz <roy.franz@linaro.org>
>>>
>>> Acked-by: Ian Campbell <ian.campbell@.citrix.com>
>>>
>>> But I do wonder why we don't just drop the hos /chosen node -- I think
>>> everything which should be in there is fabricated by the h/visor anyway.
>>
>> I though about the same things yesterday, but the /chosen node may
>> contain other properties (such as OS specific) that we may need to pass
>> to DOM0.
> 
> What sort of thing are you thinking of here?

FreeBSD provides two properties (stdin and stdout) to choose the input
and the output methods.

Those properties need to be copied in DOM0 DT to allow the user chose
which console he wants to use (think about a platform with 2 serial ports).

Regards,
Ian Campbell Oct. 14, 2014, 1:07 p.m. UTC | #8
On Tue, 2014-10-14 at 13:58 +0100, Julien Grall wrote:
> On 10/14/2014 01:49 PM, Ian Campbell wrote:
> > On Tue, 2014-10-14 at 13:28 +0100, Julien Grall wrote:
> >> Hi Ian,
> >>
> >> On 10/14/2014 08:21 AM, Ian Campbell wrote:
> >>> On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
> >>>> On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
> >>>>> (CC vijay)
> >>>>>
> >>>>> Hi Suravee,
> >>>>>
> >>>>> On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
> >>>>>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>>>>>
> >>>>>> Since EFI is not yet supported in dom0, we need to remove the following
> >>>>>> properties from the chosen node:
> >>>>>>
> >>>>>>   * linux,uefi-mmap-start
> >>>>>>   * linux,uefi-mmap-size
> >>>>>>   * linux,uefi-mmap-desc-size
> >>>>>>   * linux,uefi-mmap-desc-ver
> >>>>>>
> >>>>>> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
> >>>>>> and used by dom0 kernel to enable EFI.
> >>>>>>
> >>>>>> Cc: Julien Grall <julien.grall@linaro.org>
> >>>>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>>>>> Cc: Ian Campbell <ian.campbell@citrix.com>
> >>>>>> Cc: Roy Franz <roy.franz@linaro.org>
> >>>>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >>>>>
> >>>>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> >>>>
> >>>> Reviewed-by: Roy Franz <roy.franz@linaro.org>
> >>>
> >>> Acked-by: Ian Campbell <ian.campbell@.citrix.com>
> >>>
> >>> But I do wonder why we don't just drop the hos /chosen node -- I think
> >>> everything which should be in there is fabricated by the h/visor anyway.
> >>
> >> I though about the same things yesterday, but the /chosen node may
> >> contain other properties (such as OS specific) that we may need to pass
> >> to DOM0.
> > 
> > What sort of thing are you thinking of here?
> 
> FreeBSD provides two properties (stdin and stdout) to choose the input
> and the output methods.
> 
> Those properties need to be copied in DOM0 DT to allow the user chose
> which console he wants to use (think about a platform with 2 serial ports).

How do you know whether stdin/stdout were intended to apply to dom0 or
Xen?

What if dom0 wants to use hvc0? How is that expressed?

It seems to me that if bootargs is being consumed by Xen then
stdin/stdout ought to be consumed by Xen for consistency.

Which points to special handling for stdin/stdout in Xen, i.e. a
continuation of the whitelisting approach.

Ian.
Julien Grall Oct. 14, 2014, 1:18 p.m. UTC | #9
On 10/14/2014 02:07 PM, Ian Campbell wrote:
> On Tue, 2014-10-14 at 13:58 +0100, Julien Grall wrote:
>> On 10/14/2014 01:49 PM, Ian Campbell wrote:
>>> On Tue, 2014-10-14 at 13:28 +0100, Julien Grall wrote:
>>>> Hi Ian,
>>>>
>>>> On 10/14/2014 08:21 AM, Ian Campbell wrote:
>>>>> On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
>>>>>> On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
>>>>>>> (CC vijay)
>>>>>>>
>>>>>>> Hi Suravee,
>>>>>>>
>>>>>>> On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
>>>>>>>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>>>>>>>
>>>>>>>> Since EFI is not yet supported in dom0, we need to remove the following
>>>>>>>> properties from the chosen node:
>>>>>>>>
>>>>>>>>   * linux,uefi-mmap-start
>>>>>>>>   * linux,uefi-mmap-size
>>>>>>>>   * linux,uefi-mmap-desc-size
>>>>>>>>   * linux,uefi-mmap-desc-ver
>>>>>>>>
>>>>>>>> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
>>>>>>>> and used by dom0 kernel to enable EFI.
>>>>>>>>
>>>>>>>> Cc: Julien Grall <julien.grall@linaro.org>
>>>>>>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>>>>>> Cc: Ian Campbell <ian.campbell@citrix.com>
>>>>>>>> Cc: Roy Franz <roy.franz@linaro.org>
>>>>>>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>>>>>>
>>>>>>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
>>>>>>
>>>>>> Reviewed-by: Roy Franz <roy.franz@linaro.org>
>>>>>
>>>>> Acked-by: Ian Campbell <ian.campbell@.citrix.com>
>>>>>
>>>>> But I do wonder why we don't just drop the hos /chosen node -- I think
>>>>> everything which should be in there is fabricated by the h/visor anyway.
>>>>
>>>> I though about the same things yesterday, but the /chosen node may
>>>> contain other properties (such as OS specific) that we may need to pass
>>>> to DOM0.
>>>
>>> What sort of thing are you thinking of here?
>>
>> FreeBSD provides two properties (stdin and stdout) to choose the input
>> and the output methods.
>>
>> Those properties need to be copied in DOM0 DT to allow the user chose
>> which console he wants to use (think about a platform with 2 serial ports).
> 
> How do you know whether stdin/stdout were intended to apply to dom0 or
> Xen?

We specify the console for Xen via the Xen command line.

> What if dom0 wants to use hvc0? How is that expressed?

For now it has the priority against anything else. But in ideal world,
the HVC console should be describe in a device node add by Xen.

> It seems to me that if bootargs is being consumed by Xen then
> stdin/stdout ought to be consumed by Xen for consistency.

stdin/stdout is part of FreeBSD command line. If we use those
properties, why don't we parse Linux command line to get the console?

> Which points to special handling for stdin/stdout in Xen, i.e. a
> continuation of the whitelisting approach.

And how do you chose the console for DOM0? For Linux we have to use
console=hvc0 on the command line. FreeBSD choose to use stdin/stdout.

Regards,
Ian Campbell Oct. 14, 2014, 1:47 p.m. UTC | #10
On Tue, 2014-10-14 at 14:18 +0100, Julien Grall wrote:
> On 10/14/2014 02:07 PM, Ian Campbell wrote:
> > On Tue, 2014-10-14 at 13:58 +0100, Julien Grall wrote:
> >> On 10/14/2014 01:49 PM, Ian Campbell wrote:
> >>> On Tue, 2014-10-14 at 13:28 +0100, Julien Grall wrote:
> >>>> Hi Ian,
> >>>>
> >>>> On 10/14/2014 08:21 AM, Ian Campbell wrote:
> >>>>> On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
> >>>>>> On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall <julien.grall@linaro.org> wrote:
> >>>>>>> (CC vijay)
> >>>>>>>
> >>>>>>> Hi Suravee,
> >>>>>>>
> >>>>>>> On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com wrote:
> >>>>>>>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> >>>>>>>>
> >>>>>>>> Since EFI is not yet supported in dom0, we need to remove the following
> >>>>>>>> properties from the chosen node:
> >>>>>>>>
> >>>>>>>>   * linux,uefi-mmap-start
> >>>>>>>>   * linux,uefi-mmap-size
> >>>>>>>>   * linux,uefi-mmap-desc-size
> >>>>>>>>   * linux,uefi-mmap-desc-ver
> >>>>>>>>
> >>>>>>>> These are added by "arch/arm/efi/efi-boot.h: fdt_add_uefi_nodes()",
> >>>>>>>> and used by dom0 kernel to enable EFI.
> >>>>>>>>
> >>>>>>>> Cc: Julien Grall <julien.grall@linaro.org>
> >>>>>>>> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> >>>>>>>> Cc: Ian Campbell <ian.campbell@citrix.com>
> >>>>>>>> Cc: Roy Franz <roy.franz@linaro.org>
> >>>>>>>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> >>>>>>>
> >>>>>>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> >>>>>>
> >>>>>> Reviewed-by: Roy Franz <roy.franz@linaro.org>
> >>>>>
> >>>>> Acked-by: Ian Campbell <ian.campbell@.citrix.com>
> >>>>>
> >>>>> But I do wonder why we don't just drop the hos /chosen node -- I think
> >>>>> everything which should be in there is fabricated by the h/visor anyway.
> >>>>
> >>>> I though about the same things yesterday, but the /chosen node may
> >>>> contain other properties (such as OS specific) that we may need to pass
> >>>> to DOM0.
> >>>
> >>> What sort of thing are you thinking of here?
> >>
> >> FreeBSD provides two properties (stdin and stdout) to choose the input
> >> and the output methods.
> >>
> >> Those properties need to be copied in DOM0 DT to allow the user chose
> >> which console he wants to use (think about a platform with 2 serial ports).
> > 
> > How do you know whether stdin/stdout were intended to apply to dom0 or
> > Xen?
> 
> We specify the console for Xen via the Xen command line.
> 
> > What if dom0 wants to use hvc0? How is that expressed?
> 
> For now it has the priority against anything else. But in ideal world,
> the HVC console should be describe in a device node add by Xen.
> 
> > It seems to me that if bootargs is being consumed by Xen then
> > stdin/stdout ought to be consumed by Xen for consistency.
> 
> stdin/stdout is part of FreeBSD command line. If we use those
> properties, why don't we parse Linux command line to get the console?
> 

OK. I'm confused. I was talking about device-tree properties
under /chosen so I thought you were saying that stdin/stdout were such
things. If they are part of the bsd command line (i.e. part of
xen,dom0-bootargs or similar) then there is no problem -- we pass them
through just like we do today.

> > Which points to special handling for stdin/stdout in Xen, i.e. a
> > continuation of the whitelisting approach.
> 
> And how do you chose the console for DOM0? For Linux we have to use
> console=hvc0 on the command line. FreeBSD choose to use stdin/stdout.

Are you now saying that they aren't in the FreeBSD command line but are
properties?

If you are saying that they are dt properties then stdin/stdout should
be handled just like bootargs IMHO, not simply happen to work due to a
partial blacklist

Ian.
Julien Grall Oct. 14, 2014, 2:02 p.m. UTC | #11
On 10/14/2014 02:47 PM, Ian Campbell wrote:
> On Tue, 2014-10-14 at 14:18 +0100, Julien Grall wrote:
>> On 10/14/2014 02:07 PM, Ian Campbell wrote:
>>> On Tue, 2014-10-14 at 13:58 +0100, Julien Grall wrote:
>>>> On 10/14/2014 01:49 PM, Ian Campbell wrote:
>>>>> On Tue, 2014-10-14 at 13:28 +0100, Julien Grall wrote:
>>>>>> Hi Ian,
>>>>>> 
>>>>>> On 10/14/2014 08:21 AM, Ian Campbell wrote:
>>>>>>> On Mon, 2014-10-13 at 12:37 -0700, Roy Franz wrote:
>>>>>>>> On Mon, Oct 13, 2014 at 9:20 AM, Julien Grall
>>>>>>>> <julien.grall@linaro.org> wrote:
>>>>>>>>> (CC vijay)
>>>>>>>>> 
>>>>>>>>> Hi Suravee,
>>>>>>>>> 
>>>>>>>>> On 10/13/2014 05:17 PM, suravee.suthikulpanit@amd.com
>>>>>>>>> wrote:
>>>>>>>>>> From: Suravee Suthikulpanit
>>>>>>>>>> <Suravee.Suthikulpanit@amd.com>
>>>>>>>>>> 
>>>>>>>>>> Since EFI is not yet supported in dom0, we need to
>>>>>>>>>> remove the following properties from the chosen
>>>>>>>>>> node:
>>>>>>>>>> 
>>>>>>>>>> * linux,uefi-mmap-start * linux,uefi-mmap-size *
>>>>>>>>>> linux,uefi-mmap-desc-size *
>>>>>>>>>> linux,uefi-mmap-desc-ver
>>>>>>>>>> 
>>>>>>>>>> These are added by "arch/arm/efi/efi-boot.h:
>>>>>>>>>> fdt_add_uefi_nodes()", and used by dom0 kernel to
>>>>>>>>>> enable EFI.
>>>>>>>>>> 
>>>>>>>>>> Cc: Julien Grall <julien.grall@linaro.org> Cc:
>>>>>>>>>> Stefano Stabellini
>>>>>>>>>> <stefano.stabellini@eu.citrix.com> Cc: Ian Campbell
>>>>>>>>>> <ian.campbell@citrix.com> Cc: Roy Franz
>>>>>>>>>> <roy.franz@linaro.org> Signed-off-by: Suravee
>>>>>>>>>> Suthikulpanit <suravee.suthikulpanit@amd.com>
>>>>>>>>> 
>>>>>>>>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
>>>>>>>> 
>>>>>>>> Reviewed-by: Roy Franz <roy.franz@linaro.org>
>>>>>>> 
>>>>>>> Acked-by: Ian Campbell <ian.campbell@.citrix.com>
>>>>>>> 
>>>>>>> But I do wonder why we don't just drop the hos /chosen
>>>>>>> node -- I think everything which should be in there is
>>>>>>> fabricated by the h/visor anyway.
>>>>>> 
>>>>>> I though about the same things yesterday, but the /chosen
>>>>>> node may contain other properties (such as OS specific)
>>>>>> that we may need to pass to DOM0.
>>>>> 
>>>>> What sort of thing are you thinking of here?
>>>> 
>>>> FreeBSD provides two properties (stdin and stdout) to choose
>>>> the input and the output methods.
>>>> 
>>>> Those properties need to be copied in DOM0 DT to allow the user
>>>> chose which console he wants to use (think about a platform
>>>> with 2 serial ports).
>>> 
>>> How do you know whether stdin/stdout were intended to apply to
>>> dom0 or Xen?
>> 
>> We specify the console for Xen via the Xen command line.
>> 
>>> What if dom0 wants to use hvc0? How is that expressed?
>> 
>> For now it has the priority against anything else. But in ideal
>> world, the HVC console should be describe in a device node add by
>> Xen.
>> 
>>> It seems to me that if bootargs is being consumed by Xen then 
>>> stdin/stdout ought to be consumed by Xen for consistency.
>> 
>> stdin/stdout is part of FreeBSD command line. If we use those 
>> properties, why don't we parse Linux command line to get the
>> console?
>> 
> 
> OK. I'm confused. I was talking about device-tree properties under
> /chosen so I thought you were saying that stdin/stdout were such 
> things. If they are part of the bsd command line (i.e. part of 
> xen,dom0-bootargs or similar) then there is no problem -- we pass
> them through just like we do today.
> 
>>> Which points to special handling for stdin/stdout in Xen, i.e. a 
>>> continuation of the whitelisting approach.
>> 
>> And how do you chose the console for DOM0? For Linux we have to
>> use console=hvc0 on the command line. FreeBSD choose to use
>> stdin/stdout.
> 
> Are you now saying that they aren't in the FreeBSD command line but
> are properties?
> 
> If you are saying that they are dt properties then stdin/stdout
> should be handled just like bootargs IMHO, not simply happen to work
> due to a partial blacklist

Yes they are properties of the device tree. We can't really interpret in
Xen because they are only meaningful for FreeBSD.

On Linux, the console is chosen via the command line. It happens that we
need to clearly specify "hvc" on it to be able to use the Xen console.

As said earlier, if we consume in Xen how do you choose the console for
FreeBSD? Those properties can be considered as the part of the FreeBSD
"command line".

Anyway, the stdin/stdout properties may not be the best examples.
Grepping the Documentation in Linux I have found those properties which
live in /chosen:
	- linux,pci-probe-only
	- linux,sysrq-reset-seq (actually it's a node)

The blacklist way is the best solution to make sure any newer kernel
will work (most of the time) out of box on Linux.

Regards,
Ian Campbell Oct. 14, 2014, 2:51 p.m. UTC | #12
On Tue, 2014-10-14 at 15:02 +0100, Julien Grall wrote:
> As said earlier, if we consume in Xen how do you choose the console for
> FreeBSD? Those properties can be considered as the part of the FreeBSD
> "command line".

Like we do for bootargs, so with xen,dom0-stdin etc.

The ePAPR changelog mentions removing stdin, so I'm not sure it has
always been FreeBSD specific. I know that u-boot can also use it (that
is in the case where u-boot itself is configured with an fdt).

> Anyway, the stdin/stdout properties may not be the best examples.
> Grepping the Documentation in Linux I have found those properties which
> live in /chosen:
> 	- linux,pci-probe-only
> 	- linux,sysrq-reset-seq (actually it's a node)

Do those make any sense when running under Xen? would we want to blindly
expose them? I'm not sure.

Ian.
Julien Grall Oct. 14, 2014, 3 p.m. UTC | #13
On 10/14/2014 03:51 PM, Ian Campbell wrote:
> On Tue, 2014-10-14 at 15:02 +0100, Julien Grall wrote:
>> As said earlier, if we consume in Xen how do you choose the console for
>> FreeBSD? Those properties can be considered as the part of the FreeBSD
>> "command line".
> 
> Like we do for bootargs, so with xen,dom0-stdin etc.
> 
> The ePAPR changelog mentions removing stdin, so I'm not sure it has
> always been FreeBSD specific. I know that u-boot can also use it (that
> is in the case where u-boot itself is configured with an fdt).
> 
>> Anyway, the stdin/stdout properties may not be the best examples.
>> Grepping the Documentation in Linux I have found those properties which
>> live in /chosen:
>> 	- linux,pci-probe-only
>> 	- linux,sysrq-reset-seq (actually it's a node)
> 
> Do those make any sense when running under Xen? would we want to blindly
> expose them? I'm not sure.

For the former one, we may want to modify it if we decide to initialize
PCI devices in Xen.

For the latter, I think we need to copy as it is in Linux. This
properties seems to be used to have a set of keys to reset Linux.

Regards,
diff mbox

Patch

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 90abc3a..f1d0d20 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -423,12 +423,21 @@  static int write_properties(struct domain *d, struct kernel_info *kinfo,
          *   bootargs (from module #1, above).
          * * remove bootargs,  xen,dom0-bootargs, xen,xen-bootargs,
          *   linux,initrd-start and linux,initrd-end.
+         * * remove bootargs, linux,uefi-system-table,
+         *   linux,uefi-mmap-start, linux,uefi-mmap-size,
+         *   linux,uefi-mmap-desc-size, and linux,uefi-mmap-desc-ver
+         *   (since EFI boot is not currently supported in dom0).
          */
         if ( dt_node_path_is_equal(node, "/chosen") )
         {
             if ( dt_property_name_is_equal(prop, "xen,xen-bootargs") ||
                  dt_property_name_is_equal(prop, "linux,initrd-start") ||
-                 dt_property_name_is_equal(prop, "linux,initrd-end") )
+                 dt_property_name_is_equal(prop, "linux,initrd-end") ||
+                 dt_property_name_is_equal(prop, "linux,uefi-system-table") ||
+                 dt_property_name_is_equal(prop, "linux,uefi-mmap-start") ||
+                 dt_property_name_is_equal(prop, "linux,uefi-mmap-size") ||
+                 dt_property_name_is_equal(prop, "linux,uefi-mmap-desc-size") ||
+                 dt_property_name_is_equal(prop, "linux,uefi-mmap-desc-ver"))
                 continue;
 
             if ( dt_property_name_is_equal(prop, "xen,dom0-bootargs") )