[PATCH v4 00/47] x86: Improve operation under QEMU

[Tom Rini]

On Tue, Mar 18, 2025 at 08:32:27AM +0000, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 18 Mar 2025 at 00:05, Tom Rini <trini at konsulko.com> wrote:
> >
> > [cc list trimmed and adding Rasmus]
> >
> > On Sat, Mar 15, 2025 at 02:38:29PM +0000, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sat, 15 Mar 2025 at 13:57, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Sat, Mar 15, 2025 at 12:54:25PM +0000, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Fri, 14 Mar 2025 at 16:06, Tom Rini <trini at konsulko.com> wrote:
> > > > > >
> > > > > > On Fri, Mar 14, 2025 at 02:44:35PM +0000, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Fri, 7 Mar 2025 at 14:23, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Thu, Mar 06, 2025 at 09:03:27AM -0700, Simon Glass wrote:
> > > > > > > >
> > > > > > > > > U-Boot can start and boot an OS in both qemu-x86 and qemu-x86_64 but it
> > > > > > > > > is not perfect.
> > > > > > > > >
> > > > > > > > > With both builds, executing the VESA ROM causes an intermittent hang, at
> > > > > > > > > least on some AMD CPUs.
> > > > > > > > >
> > > > > > > > > With qemu-x86_64 kvm cannot be used since the move to long mode (64-bit)
> > > > > > > > > is done in a way that works on real hardware but not with QEMU. This
> > > > > > > > > means that performance is 4-5x slower than it could be, at least on my
> > > > > > > > > CPU.
> > > > > > > > >
> > > > > > > > > We can work around the first problem by using Bochs, which is anyway a
> > > > > > > > > better choice than VESA for QEMU. The second can be addressed by using
> > > > > > > > > the same descriptor across the jump to long mode.
> > > > > > > > >
> > > > > > > > > With an MTRR fix this allows booting into Ubuntu on qemu-x86_64
> > > > > > > > >
> > > > > > > > > In v3 some e820 patches are included to make booting reliable and avoid
> > > > > > > > > ACPI tables being dropped. Also, several MTTR problems are addressed, to
> > > > > > > > > support memory sizes above 4GB reliably.
> > > > > > > >
> > > > > > > > Do you plan to rebase the prerequisite series' this requires so that it
> > > > > > > > can be merged?
> > > > > > >
> > > > > > > Here's my understanding of where things are:
> > > > > > >
> > > > > > > 1. You rejected the membuf series and my replies to try to resolve
> > > > > > > that haven't gone anywhere yet. So your tree doesn't have any tests
> > > > > > > for that code and still has the old naming.
> > > > > >
> > > > > > https://patchwork.ozlabs.org/comment/3473898/ is a well thought out not
> > > > > > gratuitous summary of why the series as it stands is a step in the wrong
> > > > > > direction. Tests are good. They're not a reason to pull an otherwise bad
> > > > > > series. This series should be rebased to not depend on that series. The
> > > > > > tests from the other series should be split out.
> > > > >
> > > > > It's not a bad series, unfortunately. I replied with my own comments
> > > > > and I stand by them.
> > > > >
> > > > > I don't mind if you want to drop the #ifdef (which shows how a flag
> > > > > could be used and the code-size impact). But other than that, I am
> > > > > firm on this for now. I've already applied it to my tree and a membuf
> > > > > implementation with tests and without a power-of-two limitation is
> > > > > important for my current work on distros and expo.
> > > >
> > > > Well, you need to rebase this series without that then as I'm not going
> > > > to take something another long time project contributor said was wrong
> > > > and offered lots of constructive feedback on.
> > >
> > > It doesn't affect this series. It is for future work.
> > >
> > > We'll see if the LTPC replies to my concerns.
> >
> > Well, I guess I'm going to regret choosing to not chime in there at the
> > time and say that no, working against your tree is the wrong direction.
> > There's certainly enough feedback in that thread for you to make a v2
> > against mainline.  Because to be clear, no, your v1 isn't going in
> > mainline.
> 
> Firstly it does not affect this series, so I hope you can pull this
> series in soon.

A problematic side effect of your large series postings is that it ends
up being where unrelated stuff is talked about. I'm not judging v5 of
*this* series based on membuf.

> This is the first I've heard of a v2 for membuf. From my side I don't

That's what "Changes Requested" means, you need to do a vN+1 of it.

> want to rewrite it to use a different algorithm, nor force it to use a
> power-of-two size. That was not the point of my series. Here is the
> cover letter:
> 
> >>>>
> The membuff implementation curently has no tests. It also assumes that
> head and tail can never correspond unless the buffer is empty.
> 
> This series provides a compile-time flag to support a 'full' flag. It
> also adds some tests of the main routines.
> 
> The data structure is also renamed to membuf which fits better with
> U-Boot.
> 
> There may be some cases in the code which could be optimised a little,
> but the implementation is functional.
> <<<<
> 
> As I said, I'm happy to drop the addition of a compile-time flag. It
> was mostly to see if anyone thought it was a good idea.
> 
> So how about just applying the first 7 patches? I can do a revert of
> patch 8 in my tree. Then if Rasmus wants to rewrite it he can do so,
> so long as the power-of-two size isn't mandatory.

If you want to post a v2 of the membuff series against next that doesn't
contain changes that've been strongly objected to, yes, I'll review it.
No, I won't go and see if your v1 still applies to next. Given all of
the other test/ changes I would assume it doesn't.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250318/12d5f15f/attachment.sig>