Re: [PATCH v2] x86/earlyprintk: Add a force option for pciserial device

[Borislav Petkov]

On Fri, Sep 28, 2018 at 05:40:08PM +0800, Feng Tang wrote:
> "pciserial" earlyprintk helps much on many modern x86 platforms,
> but unfortunately there are still some platforms whose PCI UART
> devices have wrong PCI class code, which will be blocked by current
> class code check.
> 
> Add a option "force" so that developer could still use a UART device
> even it has wrong class code, with format "",B:D.F,force,baud". And

This new parameter and its meaning needs to be documented where
pciserial is documented.

> the original format ",B:D.F,baud" is kept unchanged.
> 
> Suggested-by: Borislav Petkov <bp@xxxxxxxxx>
> Signed-off-by: Feng Tang <feng.tang@xxxxxxxxx>
> ---
>  arch/x86/kernel/early_printk.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
> index 5e801c8..35d0f66 100644
> --- a/arch/x86/kernel/early_printk.c
> +++ b/arch/x86/kernel/early_printk.c
> @@ -213,8 +213,10 @@ static unsigned int mem32_serial_in(unsigned long addr, int offset)
>   * early_pci_serial_init()
>   *
>   * This function is invoked when the early_printk param starts with "pciserial"
> - * The rest of the param should be ",B:D.F,baud" where B, D & F describe the
> - * location of a PCI device that must be a UART device.
> + * The rest of the param should be ",B:D.F,baud" or ",B:D.F,force,baud", where
> + * B, D & F describe the location of a PCI device that must be a UART device,
> + * "force" is optional and means insisting using a UART device with a wrong
> + * pci class code.
>   */
>  static __init void early_pci_serial_init(char *s)
>  {
> @@ -224,6 +226,7 @@ static __init void early_pci_serial_init(char *s)
>  	u32 classcode, bar0;
>  	u16 cmdreg;
>  	char *e;
> +	int force = 0;
>  
>  
>  	/*
> @@ -252,6 +255,15 @@ static __init void early_pci_serial_init(char *s)
>  	if (*s == ',')
>  		s++;
>  
> +	/* User may insist to use a UART device with wrong class code */
> +	if (!strncmp(s, "force", 5)) {
> +		force = 1;
> +		s += 5;
> +
> +		if (*s == ',')
> +			s++;

No, you need to force the presence of "," otherwise cmdlines like this:

earlyprintk=pciserial,0:XX.X,forcedoodoo

work too.

> +	}
> +
>  	/*
>  	 * Second, find the device from the BDF
>  	 */
> @@ -262,10 +274,12 @@ static __init void early_pci_serial_init(char *s)
>  	/*
>  	 * Verify it is a UART type device
>  	 */
> -	if (((classcode >> 16 != PCI_CLASS_COMMUNICATION_MODEM) &&
> -	     (classcode >> 16 != PCI_CLASS_COMMUNICATION_SERIAL)) ||
> -	   (((classcode >> 8) & 0xff) != 0x02)) /* 16550 I/F at BAR0 */
> -		return;
> +	if (!force) {
> +		if (((classcode >> 16 != PCI_CLASS_COMMUNICATION_MODEM) &&
> +		     (classcode >> 16 != PCI_CLASS_COMMUNICATION_SERIAL)) ||
> +		   (((classcode >> 8) & 0xff) != 0x02)) /* 16550 I/F at BAR0 */
> +			return;

Move the force check in here ^

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.