| Message ID | 20210902021817.17506-1-chiawei_wang@aspeedtech.com |
|---|---|
| Headers | show |
| Series | arm: aspeed: Add UART routing support | expand |
Hello,
On Thu, Sep 02, 2021 at 10:18:13AM +0800, Chia-Wei Wang wrote:
> Add UART routing driver and the device tree nodes.
Thank you for working on exposing this functionality in upstreamable
way, that's so much better than all the register-level hacks in U-Boot
and similar approaches!
One (somewhat) related question that I hope you do not mind answering:
is there anything special regarding the routing or other configuration
that needs to be done for VUART to work with IRQs?
The reason I ask is that I have tried hard (and I know several other
developers who have too) to use VUART functionality but somehow as
soon as Linux was booting on host and starting to use the IRQ-based
16550 driver the communication was halted both ways. Basically, the
BMC firmware was enabling VUART in DTS, then setting LPC address to
0x3F8 and LPC IRQ to 4 and reading/writing using the corresponding
/dev/ttyS* node. The datasheet is not clearly telling what other
actions need to be performed for this to work. Not using VUART and
instead routing UART1 lines with exactly the same pinctrl/pinmux
worked just fine. One detail is that with VUART the host wasn't seeing
new interrupts but when they were simulated by exporting the LPC
interrupt pin via /sys/class/gpio and toggling it manually the data
was getting through.
Does UART1 need some explicit disabling for VUART IRQs to work? It
looks like setting LPC address and IRQ number in VUART is enough to
override the register part but probably not for the interrupt?
--
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@gmail.com
Hi Paul, > From: Paul Fertser <fercerpav@gmail.com> > Sent: Wednesday, September 8, 2021 5:43 PM > > Hello, > > On Thu, Sep 02, 2021 at 10:18:13AM +0800, Chia-Wei Wang wrote: > > Add UART routing driver and the device tree nodes. > > Thank you for working on exposing this functionality in upstreamable way, > that's so much better than all the register-level hacks in U-Boot and similar > approaches! > > > One (somewhat) related question that I hope you do not mind answering: > is there anything special regarding the routing or other configuration that > needs to be done for VUART to work with IRQs? No. The routing control has no relation to VUART. > > The reason I ask is that I have tried hard (and I know several other developers > who have too) to use VUART functionality but somehow as soon as Linux was > booting on host and starting to use the IRQ-based > 16550 driver the communication was halted both ways. Basically, the BMC > firmware was enabling VUART in DTS, then setting LPC address to > 0x3F8 and LPC IRQ to 4 and reading/writing using the corresponding > /dev/ttyS* node. The datasheet is not clearly telling what other actions need to > be performed for this to work. Not using VUART and instead routing UART1 > lines with exactly the same pinctrl/pinmux worked just fine. One detail is that > with VUART the host wasn't seeing new interrupts but when they were > simulated by exporting the LPC interrupt pin via /sys/class/gpio and toggling it > manually the data was getting through. > > Does UART1 need some explicit disabling for VUART IRQs to work? It looks like > setting LPC address and IRQ number in VUART is enough to override the > register part but probably not for the interrupt? You may need to confirm that the Host does not enable the SIO SUART1 device. This will conflict with VUART as both SUART and VAURT are competing for the port address 0x3f8 and SIRQ 4. > > -- > Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! > mailto:fercerpav@gmail.com
On Wed, Sep 08, 2021 at 10:18:35AM +0000, ChiaWei Wang wrote: > > Does UART1 need some explicit disabling for VUART IRQs to work? It looks like > > setting LPC address and IRQ number in VUART is enough to override the > > register part but probably not for the interrupt? > > You may need to confirm that the Host does not enable the SIO SUART1 > device. This will conflict with VUART as both SUART and VAURT are > competing for the port address 0x3f8 and SIRQ 4. Do you really mean the Host here, that is, software that controls LPC master when ASpeed is used as an LPC slave? Linux driver is not doing anything special with the UART1, it's just trying to use it as if it was a hardware 16550A physical IC on I/O bus. Or do you mean the BMC software shouldn't be enabling SUART1 by making sure its clock is disabled in SCU0C? Is there anything else needed? I've tried reading the ast2500 datasheet many times but this detail seem to be missing. Is there some appnote on the topic probably? In this case do we have some way to make it an obvious error to enable both SUART1 and VUART in DTS? If they're conflicting surely there should be a way to express that? Thank you for looking into this! -- Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! mailto:fercerpav@gmail.com
On Wed, Sep 08, 2021 at 01:52:45PM +0300, Paul Fertser wrote: > In this case do we have some way to make it an obvious error to enable > both SUART1 and VUART in DTS? If they're conflicting surely there > should be a way to express that? I have to add this idea is obviously silly as we have sysfs nodes to specify both LPC I/O address and interrupt number for VUART and arbitrary numbers are allowed (so it can conflict with any of the SUART port) so there should be probably some runtime checking in VUART driver to prevent that? -- Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! mailto:fercerpav@gmail.com
Hey Paul > Or do you mean the BMC software shouldn't be enabling SUART1 by making > sure its clock is disabled in SCU0C? Is there anything else needed? > I've tried reading the ast2500 datasheet many times but this detail > seem to be missing. Is there some appnote on the topic probably? I ran into this issue a while ago. As ChiaWei explained, the issue is that there are TWO implementations of a host-facing UART in the Aspeed: 1) SuperIO (aka "SIO"), which includes two UARTS (and some other stuff). The SIO is fully controlled by the host. The BMC has no ability to configure the SIO. Of course, it can disable it by stopping its clocks or by blocking access from the host to it. But it cannot observe or modify the settings that the host made. The SIO is functionally very similar to the SIOs that were around 20+ years ago. With that, it's the host's (i.e. BIOS's) responsibility to detect the presence of the SIO and to configure its UART with port (e.g. 0x3f8) and interrupt (e.g. 0x4). The SIO in the BMC will just take this configuration "no questions asked" and make the UART accessible on LPC for the host to communicate with. The BIOS also registers the UART in the SMBIOS and ACPI tables where the OS learns about its existence to load the correct driver. The SuperIO's UART actually generates/receives serial UART signals on a 3-wire interface (TX, RX, GND) at the configured speed. This signal can then be routed (there are a bunch of muxes in the Aspeed) either to another UART (which the BMC can control) or to a physical serial port (i.e. a D-Sub 9 or a header on the mainboard). The default configuration is for UART1 and UART2 (both of which are controlled by the SIO) to be routed to physical serial ports. The big problem with SIO in AST2400 / AST2500 is CVE-2019-6260. As I understand it, in order to expose the SIO to the host, a number of interfaces need to be enabled that ultimately allow the host to perform other operations that are undesirable wrt. BMC security. Please correct me if I'm wrong. 2) VUART ("Virtual UART") The VUART is "virtual", since it does not actually generate serial UART signals. Instead the VUART really is two "half UARTs" connected with each other. One half is connected to LPC to be used by the host, the other is accessible from the BMC. They both expose the UART programming interface which allows both the host and the BMC to use standard serial port drivers. There's no configurable routing for the VUART. Whatever the "port speed" that's configured by host or BMC, the VUART always runs at "maximum speed", i.e. bytes written to the UART on one end are accessible "immediately" on the other end (of course there's a maximum "baud rate" there, but that's not due to an actual serial UART signal. The big difference with VUART is that the "host settings" (i.e. port and interrupt) are fully controlled by the BMC. This requires a change in the host's BIOS, as it should no longer attempt to detect and configure the SIO and instead just expect a UART to be present at a predetermined address. For the BIOS that I worked with, this required to build and include a different module that performs the necessary initialization on the host side (e.g. configure the LPC bridge to forward requests to the specified port and accept an incoming interrupt) and to store the serial port in the SMBIOS and ACPI tables for the OS to "detect" it from. Now, if SIO is enabled on the BMC side and the host side AND if also VUART is enabled and configured on the BMC side, this may result in TWO devices listening on the port and possibly attempting to issue interrupts. I'm not sure what mechanism the Aspeed uses to prevent both the SIO's UART and the VUART to respond at the same time on the same address. Also note that there's host-side setting for LPC interrupts ("LPC IRQSER"), which can either be "quiet" or "continuous" mode, with a HW default on the host of "quiet" mode; see Intel C620 datasheet, p. 1544, when setting the Enable (EN, bit 7) bit in the Serial IRQ Control register (SCNT, offset 0x64), the Mode (MD, bit 6) bit should be set to enable "continuous mode". However, the Aspeed's default is "continuous" (not sure if it supports "quiet" mode). This results in the Aspeed not being able to send interrupts to the host. > In this case do we have some way to make it an obvious error to enable > both SUART1 and VUART in DTS? If they're conflicting surely there > should be a way to express that? No, I don't think we should try to employ any "smarts" here, as ultimately the behavior is caused by a combination of enabled BMC features (SIO) and host (BIOS) behavior. I hope that clears it up a little? I'm happy to explain more if needed. Thanks Oskar.
The patches are tested on meta-g220a build and it works fine with some changes in the sysfs path[1]. Tested-by: Lei YU <yulei.sh@bytedance.com> [1]: https://github.com/openbmc/meta-bytedance/blob/master/meta-g220a/recipes-phosphor/console/obmc-console/obmc-console%40.service#L7-L10 On Thu, Sep 2, 2021 at 10:20 AM Chia-Wei Wang <chiawei_wang@aspeedtech.com> wrote: > > Add UART routing driver and the device tree nodes. > > v2: > - Add dt-bindings > - Add ABI documents for the exported sysfs interface > - Revise driver implementation suggested by Joel > > Chia-Wei Wang (3): > dt-bindings: aspeed-lpc: Add UART routing compatible string > soc: aspeed: Add UART routing support > ARM: dts: aspeed: Add uart routing to device tree > > .../testing/sysfs-driver-aspeed-uart-routing | 15 + > .../devicetree/bindings/mfd/aspeed-lpc.txt | 22 + > arch/arm/boot/dts/aspeed-g5.dtsi | 6 + > arch/arm/boot/dts/aspeed-g6.dtsi | 6 + > drivers/soc/aspeed/Kconfig | 10 + > drivers/soc/aspeed/Makefile | 9 +- > drivers/soc/aspeed/aspeed-uart-routing.c | 601 ++++++++++++++++++ > 7 files changed, 665 insertions(+), 4 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-driver-aspeed-uart-routing > create mode 100644 drivers/soc/aspeed/aspeed-uart-routing.c > > -- > 2.17.1 > -- BRs, Lei YU
Hi Lei, Thanks for the Tested-by tag. Will include it in the upcoming v3 patch. > From: Lei Yu <yulei.sh@bytedance.com> > Sent: Thursday, September 9, 2021 2:52 PM > > The patches are tested on meta-g220a build and it works fine with some > changes in the sysfs path[1]. > > Tested-by: Lei YU <yulei.sh@bytedance.com> > > [1]: > https://github.com/openbmc/meta-bytedance/blob/master/meta-g220a/recip > es-phosphor/console/obmc-console/obmc-console%40.service#L7-L10 > > On Thu, Sep 2, 2021 at 10:20 AM Chia-Wei Wang > <chiawei_wang@aspeedtech.com> wrote: > > > > Add UART routing driver and the device tree nodes. > > > > v2: > > - Add dt-bindings > > - Add ABI documents for the exported sysfs interface > > - Revise driver implementation suggested by Joel > > > > Chia-Wei Wang (3): > > dt-bindings: aspeed-lpc: Add UART routing compatible string > > soc: aspeed: Add UART routing support > > ARM: dts: aspeed: Add uart routing to device tree > > > > .../testing/sysfs-driver-aspeed-uart-routing | 15 + > > .../devicetree/bindings/mfd/aspeed-lpc.txt | 22 + > > arch/arm/boot/dts/aspeed-g5.dtsi | 6 + > > arch/arm/boot/dts/aspeed-g6.dtsi | 6 + > > drivers/soc/aspeed/Kconfig | 10 + > > drivers/soc/aspeed/Makefile | 9 +- > > drivers/soc/aspeed/aspeed-uart-routing.c | 601 > ++++++++++++++++++ > > 7 files changed, 665 insertions(+), 4 deletions(-) create mode > > 100644 Documentation/ABI/testing/sysfs-driver-aspeed-uart-routing > > create mode 100644 drivers/soc/aspeed/aspeed-uart-routing.c > > > > -- > > 2.17.1 > > > > > -- > BRs, > Lei YU