Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 17 Mar 2020, Julien Grall wrote:
> Hi,
> 
> On 13/03/2020 23:14, Stefano Stabellini wrote:
> > On Fri, 13 Mar 2020, Stefano Stabellini wrote:
> > > On Mon, 9 Mar 2020, Anthony PERARD wrote:
> > > > At the moment, early printk can only be configured on the make command
> > > > line. It is not very handy because a user has to remove the option
> > > > everytime it is using another command other than compiling the
> > > > hypervisor.
> > > > 
> > > > Furthermore, early printk is one of the few odds one that are not
> > > > using Kconfig.
> > > > 
> > > > So this is about time to move it to Kconfig.
> > > > 
> > > > The new kconfigs options allow a user to eather select a UART driver
> > > > to use at boot time, and set the parameters, or it is still possible
> > > > to select a platform which will set the parameters.
> > > > 
> > > > If CONFIG_EARLY_PRINTK is present in the environment or on the make
> > > > command line, make will return an error.
> > > > 
> > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > > > 
> > > > ---
> > > > 
> > > > Original patch:
> > > >      [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early
> > > > printk using Kconfig
> > > >      <20190913103953.8182-1-julien.grall@xxxxxxx>
> > > > ---
> > > > 
> > > > Notes:
> > > >      v3:
> > > >      - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
> > > >        select which object to build).
> > > >      - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
> > > >      - typos
> > > >      - drop the list of aliases in early-printk.txt. Kconfig choice menu
> > > >        should be enough.
> > > >      - reword early-printk.txt.
> > > >      - rework how EARLY_PRINTK is set to Y
> > > >        and use that instead of a list of all EARLY_UART_*
> > > >      - Add a check to ask user to use Kconfig to set early printk.
> > > >      - rework the possible choice to have all uart driver and platform
> > > >        specific option together.
> > > >      - have added or reword prompt and help messages of the different
> > > >        options. The platform specific option don't have extended help,
> > > > the
> > > >        prompt is probably enough.
> > > >        (The non-platform specific options have the help message that
> > > > Julien
> > > >        have written in the first version.)
> > > >      - have made EARLY_UART_INIT dependent on the value of
> > > >        EARLY_UART_PL011_BAUD_RATE so that there is no need to expose
> > > > _INIT to
> > > >        users.
> > > > 
> > > 
> > > The patch is fine by me. I only have one very minor comment below.
> > 
> > I forgot to add
> > 
> > Tested-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> 
> Thank you for the testing. This patch contains a question for you (see [1]).
> Can you please take a look?
> 
> Cheers,
> 
> [1] <7720f54e-7ef7-9dae-dfdb-38b89a0892a6@xxxxxxx>

So the question is:

> I noticed below you added "depends on ARM_64" on the Xilinx SoC. In 
> general, platform specific options are tied to either arm32 or arm64, 
> even if the UART "driver" is arch agnostic.
> 
> You could technically boot Xen on Arm 32-bit on Armv8 HW provided they 
> support 32-bit at the hypervisor level, but we never supported this 
> case. So I am wondering whether we should add depends on each 
> earlyprintk. Stefano, any opinions?

Well spotted.

Xilinx doesn't support 32-bit Xen on their boards, "support" as in test,
run or validate. So it would not be a problem from Xilinx point of view
to add a "depends on ARM_64".

I take that you are suggesting adding "depends on ARM_64/32" under the
legacy platform earlyprintk options, from EARLY_PRINTK_BRCM to
EARLY_PRINTK_ZYNQMP right? If so, I am fine with it, and it seems like a
good idea.

The other new generic earlyprintk options, the ones that only depend on
the uart driver, from EARLY_UART_CHOICE_8250 to EARLY_UART_CHOICE_SCIF,
it feels more natural to leave them without a specific arch dependency.

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