Messages in this thread | | | Date | Fri, 28 Sep 2018 09:47:39 +0800 | From | Feng Tang <> | Subject | Re: [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device with incorrect class code |
| |
Hi Borislav,
Thanks for the review.
On Thu, Sep 27, 2018 at 03:30:50PM +0200, Borislav Petkov wrote: > On Thu, Sep 27, 2018 at 08:43:20PM +0800, Feng Tang wrote: > > "pciserial" earlyprintk helps much on many modern x86 platforms, > > but unfortunately there are some platforms whose PCI UART devices > > have wrong PCI class code, which will be blocked by current check. > > > > So loose the class code check by giving a warning message instead. > > This should be fine, as a developer who can give the accurate > > BDF should know whether it's a usable UART device. > > BDF? No. > > Please write it out what it means.
Will do.
> > Signed-off-by: Feng Tang <feng.tang@intel.com> > > --- > > arch/x86/kernel/early_printk.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c > > index 5e801c8..abe1d08 100644 > > --- a/arch/x86/kernel/early_printk.c > > +++ b/arch/x86/kernel/early_printk.c > > @@ -265,7 +265,8 @@ static __init void early_pci_serial_init(char *s) > > if (((classcode >> 16 != PCI_CLASS_COMMUNICATION_MODEM) && > > (classcode >> 16 != PCI_CLASS_COMMUNICATION_SERIAL)) || > > (((classcode >> 8) & 0xff) != 0x02)) /* 16550 I/F at BAR0 */ > > - return; > > + pr_warn("earlyprintk: classcode for pcidev %d:%d:%d shows it's not a UART like device, please check!\n", > > + bus, slot, func); > > So where did the return statement go? > > What are you trying to do here? If the device is still an UART device > then we don't need the check at all as you're basically overriding it > and only the class code is wrong. > > If so, why do we need the pr_warn at all? What can the user do about the > class code? Nothing, I'd say. I don't see her "fixing" PCI config space > so that the device has a correct class code.
That's true. We can hardly do anything from OS side,
> > But what happens if the user really supplies the wrong BDF? We end up > poking in PCI config space of *some* device and then the cat might catch > fire.
Valid point, I can not argue on this :)
> > So it sounds to me like we need: > > earlyprintk=pciserial,force,00:18.1,115200
I would follow this way, thanks
-Feng
| |