Re: [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device with incorrect class code
From: Feng Tang
Date: Thu Sep 27 2018 - 21:42:34 EST
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.
> > Signed-off-by: Feng Tang <feng.tang@xxxxxxxxx>
> > ---
> > 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
Valid point, I can not argue on this :)
> So it sounds to me like we need:
I would follow this way, thanks