From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1lNHwS-0000PZ-JP for mharc-grub-devel@gnu.org; Fri, 19 Mar 2021 12:25:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:50610) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lNHwK-0000NO-Bp for grub-devel@gnu.org; Fri, 19 Mar 2021 12:25:33 -0400 Received: from mail-oi1-x234.google.com ([2607:f8b0:4864:20::234]:35412) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lNHwI-00079v-98 for grub-devel@gnu.org; Fri, 19 Mar 2021 12:25:32 -0400 Received: by mail-oi1-x234.google.com with SMTP id x2so5354347oiv.2 for ; Fri, 19 Mar 2021 09:25:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references:reply-to :mime-version:content-transfer-encoding; bh=SMXL/TzzXSVt9f86S+PZuvb/epB3hqRObJvNyxtHPCQ=; b=0uykIxjuuqBVbVEP6a/CsvOCxC0abIwK7THXIzesDneagO/XJ2viwVoyEQqZatd7mS b2wdpDcbGvRPpZefFgP3yGkmZD6Puu1vYS8RazUbVOn8GsjUO4b1iixegjslFHCKJpB0 TTkfVpg4oH+XGbCCGX4Uas6y75UyQv6xLTsekIBqOnoJB5VsmeC7iyWZvwTZevuHdy6I AGpPqz8B2zXhh57e/Hk0QIznlq9xz3aSVhEpo3hiQ5+WSJFp1RNuzsbQT9Eejh+M+hXF xwiOp5Wctdi2ffuQmkU86e2c5XvUwDlgtbY0omdxdRN24T8jXbKeX32jp6oF0OTOGlGD zCZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:reply-to:mime-version:content-transfer-encoding; bh=SMXL/TzzXSVt9f86S+PZuvb/epB3hqRObJvNyxtHPCQ=; b=D4gS9n8Uaqy7UQW2qsqWf3mtH0ckGJ54XD5gg0QFGbflq4YcjokA95NivKJK+Z0mJs PnLJVlOZvB6BJGdyVJFVgmS11WwkHwfQT4U7Xyr7FAL0xxrZ0GZgRNcUKDdiljl9oizJ b78vkMW4kdYW/xtg3T5hfQw1//nXlxztEbR1LnwmljT8BQCj6J5W4GXlOpA9kt0UOfEg WB/eNtP84opjcDHupFrTSQ23MJd+pgdXjekFQmQu6f47vhWWeC5mray19c03p1g0PMB3 meVC+/dbXRanoF6g/y2RezT5R8NINSVDfbLTo0olD8UX9UHPXCNOAI7Cj9qEMbPuzfjA DcGw== X-Gm-Message-State: AOAM531h8vFv570vrKCzI+WWhlcUWj0rpc47hR8QiUPkqIp15QZoybE4 oe/JGaqujnmtDR4gSARQNT4iOA== X-Google-Smtp-Source: ABdhPJyqQLefPkO+UoeoIagaPCb7mLtK0UML/ZbYFy3qlxsbJR1YUx1eWWweO4CymRaea7BgKh4zWA== X-Received: by 2002:a05:6808:3dc:: with SMTP id o28mr1647186oie.120.1616171129011; Fri, 19 Mar 2021 09:25:29 -0700 (PDT) Received: from crass-HP-ZBook-15-G2 ([2605:a601:ab16:db00:2328:d973:f35b:5c6b]) by smtp.gmail.com with ESMTPSA id 3sm1342663ood.46.2021.03.19.09.25.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Mar 2021 09:25:28 -0700 (PDT) Date: Fri, 19 Mar 2021 11:25:18 -0500 From: Glenn Washburn To: Benjamin Herrenschmidt Cc: The development of GNU GRUB , Matthias Lange Subject: Re: [PATCH 0/5] serial: Add MMIO & SPCR support for AWS EC2 metal instances Message-ID: <20210319112518.09595b43@crass-HP-ZBook-15-G2> In-Reply-To: References: <20210318220728.495970-1-benh@kernel.crashing.org> <20210318190346.11570717@crass-HP-ZBook-15-G2> Reply-To: development@efficientek.com X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=2607:f8b0:4864:20::234; envelope-from=development@efficientek.com; helo=mail-oi1-x234.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Mar 2021 16:25:34 -0000 On Fri, 19 Mar 2021 12:27:15 +1100 Benjamin Herrenschmidt 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