Hi Borislav,

On Sat, Sep 29, 2018 at 06:34:58PM +0200, Borislav Petkov wrote:
> 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.

Ok, will change the kernel-parameters.txt in next version.

> 
> > the original format ",B:D.F,baud" is kept unchanged.
> > 
> > Suggested-by: Borislav Petkov <[email protected]>
> > Signed-off-by: Feng Tang <[email protected]>
> > ---
> >  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.

Got your point. As the following [,baudrate] is also optional, and this
"force" could be the last option for pciserial. The check may be

        if (!strncmp(s, "force,", 6) || !strncmp(s, "force ", 6)) {
                force = 1;
                s += 6;
        }


> > +   }
> > +
> >     /*
> >      * 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 ^

Do you mean this? 

        if (((classcode >> 16 != PCI_CLASS_COMMUNICATION_MODEM) &&
             (classcode >> 16 != PCI_CLASS_COMMUNICATION_SERIAL)) ||
           (((classcode >> 8) & 0xff) != 0x02)) /* 16550 I/F at BAR0 */ {
                if (!force)
                        return;
        }

Thanks,
Feng


Reply via email to