[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/5] serial: Add MMIO & SPCR support for AWS EC2 metal instan
From: |
Glenn Washburn |
Subject: |
Re: [PATCH 0/5] serial: Add MMIO & SPCR support for AWS EC2 metal instances |
Date: |
Fri, 19 Mar 2021 11:25:18 -0500 |
On Fri, 19 Mar 2021 12:27:15 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Thu, 2021-03-18 at 19:03 -0500, Glenn Washburn wrote:
> > > 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).
>
> Yes, I added an "mmio" option to pci-serial that makes register with
> the Amazon UART vid/did and use an MMIO BAR :-) It's a bit of a hack
> but I can try upstreaming it.
I may have been confusing above. When I said deal breaker, I didn't
mean for the patch series. I meant if it was source changes to qemu (as
it is), it would be a deal breaker to require that test. It would be
ideal to try to upstream the change to qemu and then get a test working
here, but it shouldn't be a requirement to include this patch series.
> > 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).
>
> My experience with ACPI is extremely limited, I've never done such
> things as build tables etc... but I can certainly try to look into it
> as time permits.
Ok, I was hoping it might be low hanging fruit. As it seems like it
might not be, I don't think not having this test should be a blocker.
But it would be nice if you could take a quick look at it to see if it
would in fact be easy.
> > Also, could you add to the documentation on the usage of these
> > changes?
>
> I added some documentation to the "serial" command syntax change for
> MMIO. I didn't add anything about SPCR indeed, where do you suggest I
> add it ? Same spot ?
Yes, at the end of that paragraph an additional sentence seems like a
good place. I'll add a comment on some changes to the --mmio doc change
in that patch.
> > New functionality should in general come with new tests (when
> > feasible)
>
> Yes, I understand, it's just that in this specific case it's rather
> hard ... :-) I'll see what I can do with qemu but the best case
> scenario would involve upstreaming something there and then depending
> on that change trickling down to be able to use it in the grub tests
> which would be ... tricky....
Yep, I'm not suggesting we add any tests that rely on changes not in
a qemu release.
> > and additions to the documentation.
>
> Right, I did a bit, I can look into adding more, let me know if there
> are other parts of the doc you wish me to update.
Ok, so to sum up, I don't think these changes should be blocked by not
having tests. There still needs to be a proper code review, which I'm
no comfortable doing since its not really my area.
Glenn
- [PATCH 0/5] serial: Add MMIO & SPCR support for AWS EC2 metal instances, Benjamin Herrenschmidt, 2021/03/18
- [PATCH 1/5] acpi: Export a generic grub_acpi_find_table, Benjamin Herrenschmidt, 2021/03/18
- [PATCH 2/5] acpi: Add SPCR and generic address definitions, Benjamin Herrenschmidt, 2021/03/18
- [PATCH 5/5] ns8250: Use ACPI SPCR table when available to configure serial, Benjamin Herrenschmidt, 2021/03/18
- [PATCH 3/5] ns8250: Add base support for MMIO UARTs, Benjamin Herrenschmidt, 2021/03/18
- [PATCH 4/5] ns8250: Add configuration parameter when adding ports, Benjamin Herrenschmidt, 2021/03/18
- Re: [PATCH 0/5] serial: Add MMIO & SPCR support for AWS EC2 metal instances, Glenn Washburn, 2021/03/18
- Re: [PATCH 0/5] serial: Add MMIO & SPCR support for AWS EC2 metal instances, Daniel Kiper, 2021/03/23