* [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device with incorrect class code @ 2018-09-27 12:43 Feng Tang 2018-09-27 13:30 ` Borislav Petkov 0 siblings, 1 reply; 3+ messages in thread From: Feng Tang @ 2018-09-27 12:43 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Borislav Petkov, Peter Zijlstra, Stuart R . Anderson, alan, x86, linux-kernel Cc: Feng Tang "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. 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); /* * Determine if it is IO or memory mapped -- 2.7.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device with incorrect class code 2018-09-27 12:43 [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device with incorrect class code Feng Tang @ 2018-09-27 13:30 ` Borislav Petkov 2018-09-28 1:47 ` Feng Tang 0 siblings, 1 reply; 3+ messages in thread From: Borislav Petkov @ 2018-09-27 13:30 UTC (permalink / raw) To: Feng Tang Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Peter Zijlstra, Stuart R . Anderson, alan, x86, linux-kernel 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@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. 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. So it sounds to me like we need: earlyprintk=pciserial,force,00:18.1,115200 where "force" allows the function to continue even if the classcode is wrong. Or earlyprintk=pciserial,i_know_what_im_doing,00:18.1,115200 :-) Also, if you're going to use pr_* variants, convert the file to use pr_fmt() - grep the kernel sources for examples. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device with incorrect class code 2018-09-27 13:30 ` Borislav Petkov @ 2018-09-28 1:47 ` Feng Tang 0 siblings, 0 replies; 3+ messages in thread From: Feng Tang @ 2018-09-28 1:47 UTC (permalink / raw) To: Borislav Petkov Cc: Thomas Gleixner, Ingo Molnar, H Peter Anvin, Peter Zijlstra, Stuart R . Anderson, alan, x86, linux-kernel 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-09-28 1:42 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-27 12:43 [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device with incorrect class code Feng Tang 2018-09-27 13:30 ` Borislav Petkov 2018-09-28 1:47 ` Feng Tang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).