On Fri, 19 Mar 2021 12:27:15 +1100 Benjamin Herrenschmidt <b...@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 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
