Re: [PATCH 0/5] serial: Add MMIO & SPCR support for AWS EC2 metal instances

[Glenn Washburn]

Hey Ben,

On Fri, 19 Mar 2021 09:07:23 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> (Apologies if that got sent twice, there was an issue with my setup
> yesterday causing it to be sent with the wrong From: line)
> 
> This series adds support for the serial console of AWS EC2 "metal" x86
> instances to grub. This requires two improvements:
> 
> - Support for MMIO accesses to the 8250 serial port. This adds the
> basic plumbing, a function to register an MMIO based port, and
> support for configuring an MMIO port by using the string "mmio"
> followed by a hex address for the --port directive. [Q: Should we
> instead add a --mmio directive ? Can we make two such options
> mutually exclusive with the current infrastructure ?]
> 
> - Support for setting up a default port using the ACPI SPCR table if
> present. This series will make this happen if the command "serial" is
> used without arguments. In that case, SPCR will be used if present,
> otherwise grub will revert to com0 with default settings as before.
> 
> This work started originally from Matthias Lange series
> https://marc.info/?l=grub-devel&m=148775823217022&w=2
> 
> However, I ended up rewriting most of it using a different approach
> which I felt was less invasive and simpler, as I don't expect we will
> be collecting more 'backends' for ns8250.c among other things.
> 
> I did not keep Matthias original support for OXSEMI PCI uart. This can
> be fairly easily added on top, however, I believe a better approach
> would be to define a syntax to the "serial" command to define a PCI
> UART by seg/bus/dev/fn with options to set the base clock.
> 
> We could also add a table of "known" ones as we go based in vid/did
> similar to what Linux does. None of this was necessary for my purpose
> and I lack the ability/time to test this setup, but it would be easy
> to add it on top of this series.
> 
> This was tested using SPCR on an actual c5.metal instance, and using
> explicit instanciation via serial -p mmioXXXXXXXX on a modified qemu
> hacked to create MMIO PCI serial ports.

When you say a modified qemu, was that a source level change? I'm
curious how hard it would be to add this test to the current GRUB make
check tests (many of which already use qemu). Of course, if they source
of qemu was modified, then its probably a deal breaker (until it could
be accepted upstream). 

Also I haven't looked into it, but seems like it might not be hard to
add a separate test for the part using the SPCR table via qemu (perhaps
using the "-acpitable" arg).

Also, could you add to the documentation on the usage of these changes?
New functionality should in general come with new tests (when feasible)
and additions to the documentation.

Thanks for contributing,
Glenn