All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
To: Alistair Francis <alistair23@gmail.com>
Cc: "Daniel Henrique Barboza" <daniel.barboza@oss.qualcomm.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@oss.qualcomm.com>,
	qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
	alistair.francis@wdc.com, liwei1518@gmail.com,
	zhiwei_liu@linux.alibaba.com, chao.liu.zevorn@gmail.com
Subject: Re: [PATCH 00/24] target/riscv: move TCG files and fix --disable-tcg
Date: Fri, 26 Jun 2026 08:28:01 -0700	[thread overview]
Message-ID: <feb50737-7897-4881-95f1-cdf1725bb7a2@oss.qualcomm.com> (raw)
In-Reply-To: <CAKmqyKOXPCWPFP7Htr5BByp=7jsDDp6kYXRhyn6Ycre0pjofCg@mail.gmail.com>

On 6/25/2026 6:41 PM, Alistair Francis wrote:
> On Wed, Jun 24, 2026 at 5:18 AM Pierrick Bouvier
> <pierrick.bouvier@oss.qualcomm.com> wrote:
>>
>> On 6/23/2026 12:01 PM, Daniel Henrique Barboza wrote:
>>>
>>>
>>> On 6/23/2026 1:10 PM, Pierrick Bouvier wrote:
>>>> On 6/23/2026 4:38 AM, Daniel Henrique Barboza wrote:
>>>>>
>>>>>
>>>>> On 6/23/2026 6:58 AM, Peter Maydell wrote:
>>>>>> On Tue, 23 Jun 2026 at 10:49, Daniel Henrique Barboza
>>>>>> <daniel.barboza@oss.qualcomm.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 6/22/2026 6:34 PM, Pierrick Bouvier wrote:
>>>>>>>> On 6/22/2026 2:23 PM, Philippe Mathieu-Daudé wrote:
>>>>>>>>> On 22/6/26 22:52, Pierrick Bouvier wrote:
>>>>>>>>>> On 6/22/2026 12:31 PM, Daniel Henrique Barboza wrote:
>>>>>>>>>>> Hello,
>>>>>>>>>>>
>>>>>>>>>>> This series looks scary but it's mostly trivial and mechanical
>>>>>>>>>>> work.
>>>>>>>>>>>
>>>>>>>>>>> It is yet another attempt at fixing --disable-tcg.  We have a
>>>>>>>>>>> recent
>>>>>>>>>>> work sent to the ML [1] and we had Phil's attempt back in 2023
>>>>>>>>>>> [2].
>>>>>>>>>>> Phil's work didn't get merged and it's now too hard to rebase and
>>>>>>>>>>> revive, the most recent attempt got misled into the 'what is
>>>>>>>>>>> common code
>>>>>>>>>>> between TCG and KVM' dungeon.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> It seems like series does not apply on top of master, would that be
>>>>>>>>>> possible to rebase it?
>>>>>>>>>
>>>>>>>>> For some reason the RISC-V series are handled distinctly than the
>>>>>>>>> rest of QEMU, Alistair queues work on his repository and developers
>>>>>>>>> are custome to base their series on top of it (otherwise Alistair
>>>>>>>>> can not apply them on his tree and asks for reposts), see the
>>>>>>>>> riscv-to-apply.next branch on https://github.com/alistair23/qemu.
>>>>>>>>
>>>>>>>> Unfortunately, it makes it hard to run any kind of automated testing,
>>>>>>>> especially for series like this that target specific configs.
>>>>>>>
>>>>>>> Don't we have ways of saying in the commit message "these patches
>>>>>>> applies
>>>>>>> on top of these other patches" and then the tooling would deal with
>>>>>>> it?
>>>>>>> I remember patchew doing stuff like that with that "Based-on:
>>>>>>> <message-id>"
>>>>>>> tag.
>>>>>>
>>>>>> Yes, Based-on: is our convention for marking "this patchset needs some
>>>>>> other one to be applied first". But that should be the exception rather
>>>>>> than a common case -- if patchsets regularly need to be based on
>>>>>> something other than head-of-git, this is I think a sign that
>>>>>> maintainers are not sending out pull requests frequently enough.
>>>>>>
>>>>>> I would prefer it if QEMU didn't develop kernel-style "subsystems
>>>>>> have their own particular workflows" fragmentation -- I don't
>>>>>> think we're big enough or that sub-parts of QEMU are sufficiently
>>>>>> well separated for it to work out well.
>>>>>
>>>>> I agree that rebasing things on master is better than rebasing it on the
>>>>> maintainer's tree.  And we could make a better job at informing
>>>>> developers that
>>>>> submitting a patch for qemu-riscv, vfio or any particular subtree, means
>>>>> that
>>>>> the patch should be based on a maintainer tree X.
>>>>>
>>>>> The thing is that sending patches on master only works if master is
>>>>> always up
>>>>> to date, and that's not feasible with our current style of merging PRs.
>>>>> This
>>>>> series we're commenting on is an example: it doesn't apply to master
>>>>> because
>>>>> there are pre-approved RISC-V patches in the maintainer's tree from 2
>>>>> days ago
>>>>> (also my patches, I might add) that caused conflicts that I wasn't aware
>>>>> that
>>>>> would happen.  This conflict would have to be dealt with at some point
>>>>> by myself
>>>>> or the maintainer, and it's not like 2 days is too much time without
>>>>> a PR.
>>>>>
>>>>> We can argue "this is an exception that doesn't happen that often, we
>>>>> should
>>>>> stick with using master as a base", and to a certain extend that's
>>>>> true.  But
>>>>> then this sort of conflict happens again, then again, then again, it
>>>>> comes to
>>>>> a point where it's easier to tell developers to use the maintainer's
>>>>> tree instead
>>>>> of master.
>>>>>
>>>>> Maybe I'm downplaying the problem because I've been sending stuff based
>>>>> on the
>>>>> maintainer's tree since forever and got used to it.  IMO, unless we
>>>>> decide to be
>>>>> like libvirt and create the "committer" role to allow trustworthy devs
>>>>> to push
>>>>> stuff to master after acks, making it more feasible to expect master to
>>>>> be up to
>>>>> date, I'm afraid we're closer to a kernel-style workflow.  For better or
>>>>> worse.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Daniel
>>>>>
>>>>>
>>>>>>
>>>>>> thanks
>>>>>> -- PMM
>>>>>
>>>>
>>>> In this very specific case, where base patches are needed, maybe it
>>>> would be better to make the required commits appear in this series, and
>>>> mention in cover letter that patches 1-N are just coming from another
>>>> series and are already reviewed/approved. IMHO it doesn't hurt, and
>>>> reviewers are free to skip commits already reviewed.
>>>
>>> That's fair enough but I wonder if that won't scare people away with
>>> even bigger series :D  in this case here I would need to either send all
>>> the queued patches, making the series go to 40+, or I would need to triage
>>> which patches from the queue creates a conflict with this work and send
>>> only those.
>>>
>>> Now, as for qemu-ci ...  How farfetched it is to make it read a specific
>>> tag
>>> in the cover-letter, e.g. "branch-id", that can point to a gitlab/github
>>> repo with the patches, and use that code base instead of applying the
>>> patches to the master branch?  Then for the next version of this work
>>> I could do
>>>
>>> "branch-id: https://gitlab.com/danielhb/qemu/-/tree/riscv_disabletcg_v2"
>>>
>>
>> Based-on: is a QEMU specific tag, that is only understood by patchew,
>> and no other tool to my knowledge.
>> b4 has base_commit, which allows to give a specific base, but not a
>> specific repository. I'm not aware of any b4 tag that allows to mention
>> a base series. It makes sense, series are not branches, and stacking
>> them comes with a lot of problems.
>>
>> So it seems like email workflow is quite limited in this regard, and the
>> only way to deal with it properly is to wait for base patches to be
>> merged, or include them in the series.
>>
>> From another perspective, the same problem would exist if we would use a
>> forge like GitHub or GitLab. It's not possible to stack PR on top of
>> others, and only solution is to wait, or duplicate patches. IMHO, it's a
>> sane default, as it forces correct ordering instead of allowing chaotic
>> development.
>>
>>> and the tool would still work.  If there's no "branch-id" then it assumes
>>> that the patches are to be applied on master.
>>>
>>>
>>> And yeah, in an ideal world the problem goes away if we just do more PRs
>>> and
>>> strive to keep 'master' updated.  I'm just thinking out loud about possible
>>> alternatives until we reach that point.
> 
> When you sent v2 of this series it had been one week since the last
> RISC-V PR. Are we really aiming for more than one PR a week?
> 
>>>
>>>
>>> Cheers,
>>> Daniel
>>>
>>>>
>>>> Or, a solution I'm not fond of but I ended up adopting most of the time,
>>>> just wait for required patches to be merged on master before posting the
>>>> series, and work on something else meanwhile.
> 
> I do feel that a few people do that, just not Daniel :)
>

Waiting works well if you know the wait period is deterministic. "Ok, I
missed this train, let me catch the one next week". Unfortunately, it
varies per maintainer, and some are even stochastic processes on their own.

I can't blame people who submit and are not maintainer to feel
frustration with this, it's a real issue.

> Alistair
> 
>>>>
>>>> Ideally, yes, it would be better if maintainers could send PR more
>>>> frequently to avoid creating those intermediate staging trees. The
>>>> faster we merge, the less conflicts we'll have.
>>>>
>>>> Regards,
>>>> Pierrick
>>>
>>
>>



  reply	other threads:[~2026-06-26 15:28 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-22 19:31 [PATCH 00/24] target/riscv: move TCG files and fix --disable-tcg Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 01/24] target/riscv: Remove unused tcg/tcg.h include Daniel Henrique Barboza
2026-06-27 11:03   ` Chao Liu
2026-06-22 19:31 ` [PATCH 02/24] target/riscv: move TCG only files to tcg subdir Daniel Henrique Barboza
2026-06-22 20:52   ` Philippe Mathieu-Daudé
2026-06-27 11:04   ` Chao Liu
2026-06-22 19:31 ` [PATCH 03/24] target/riscv/machine.c: do not migrate pmp state with kvm Daniel Henrique Barboza
2026-06-22 21:25   ` Philippe Mathieu-Daudé
2026-06-23 10:25     ` Philippe Mathieu-Daudé
2026-06-23 14:59     ` Richard Henderson
2026-06-27 11:45   ` Chao Liu
2026-06-22 19:31 ` [PATCH 04/24] target/riscv: move pmp files to tcg subdir Daniel Henrique Barboza
2026-06-22 20:53   ` Philippe Mathieu-Daudé
2026-06-27 11:51   ` Chao Liu
2026-06-22 19:31 ` [PATCH 05/24] target/riscv: make some riscv_sysemu_ops TCG only Daniel Henrique Barboza
2026-06-22 21:30   ` Philippe Mathieu-Daudé
2026-06-23 15:02     ` Richard Henderson
2026-06-23 17:25     ` Daniel Henrique Barboza
2026-06-23 21:15       ` Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 06/24] target/riscv: move pmu.h to tcg subdir Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 07/24] target/riscv: move debug.h " Daniel Henrique Barboza
2026-06-22 20:55   ` Philippe Mathieu-Daudé
2026-06-22 19:31 ` [PATCH 08/24] target/riscv: remove csr.h from kvm-cpu.c Daniel Henrique Barboza
2026-06-22 20:57   ` Philippe Mathieu-Daudé
2026-06-22 19:31 ` [PATCH 09/24] target/riscv: move csr.h to tcg subdir Daniel Henrique Barboza
2026-06-22 20:58   ` Philippe Mathieu-Daudé
2026-06-22 19:31 ` [PATCH 10/24] target/riscv: move custom_csrs logic to tcg-cpu.c Daniel Henrique Barboza
2026-06-22 21:02   ` Philippe Mathieu-Daudé
2026-06-23 19:19     ` Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 11/24] target/riscv: move riscv_cpu_set_nmi() " Daniel Henrique Barboza
2026-06-22 21:04   ` Philippe Mathieu-Daudé
2026-06-23 19:59     ` Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 12/24] target/riscv: move valid_vm_* satp arrays to cpu.c Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 13/24] target/riscv: move some irq helpers " Daniel Henrique Barboza
2026-06-22 21:07   ` Philippe Mathieu-Daudé
2026-06-22 19:31 ` [PATCH 14/24] target/riscv: move riscv_cpu_claim_interrupts " Daniel Henrique Barboza
2026-06-22 21:08   ` Philippe Mathieu-Daudé
2026-06-22 19:31 ` [PATCH 15/24] target/riscv/cpu.c: handle TCG bits of riscv_cpu_dump_state Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 16/24] target/riscv: gate riscv_cpu_update_mip with tcg_enabled() Daniel Henrique Barboza
2026-06-22 21:12   ` Philippe Mathieu-Daudé
2026-06-23 20:40     ` Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 17/24] target/riscv/cpu.c: filter TCG only bits in riscv_cpu_reset_hold() Daniel Henrique Barboza
2026-06-22 21:13   ` Philippe Mathieu-Daudé
2026-06-24 11:19     ` Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 18/24] hw/riscv/riscv_hart.c isolate tcg only bits Daniel Henrique Barboza
2026-06-22 21:15   ` Philippe Mathieu-Daudé
2026-06-22 19:31 ` [PATCH 19/24] target/riscv/gdbstub.c: isolate TCG only checks Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 20/24] target/riscv: move riscv_cpu_set_rdtime_fn to riscv_aclint Daniel Henrique Barboza
2026-06-22 21:16   ` Philippe Mathieu-Daudé
2026-06-22 19:31 ` [PATCH 21/24] target/riscv/tcg: remove unused riscv_cpu_get_geilen() Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 22/24] target/riscv: move riscv_cpu_set_geilen() to riscv-imsic Daniel Henrique Barboza
2026-06-22 19:31 ` [PATCH 23/24] target/riscv: move riscv_cpu_set_aia_ireg_rmw_cb() to riscv_imsic Daniel Henrique Barboza
2026-06-22 21:17   ` Philippe Mathieu-Daudé
2026-06-22 19:31 ` [PATCH 24/24] gitlab-ci.d/crossbuilds: add riscv64 KVM-only build job Daniel Henrique Barboza
2026-06-22 20:52 ` [PATCH 00/24] target/riscv: move TCG files and fix --disable-tcg Pierrick Bouvier
2026-06-22 21:04   ` Daniel Henrique Barboza
2026-06-22 21:23   ` Philippe Mathieu-Daudé
2026-06-22 21:34     ` Pierrick Bouvier
2026-06-23  8:17       ` Philippe Mathieu-Daudé
2026-06-23  9:48       ` Daniel Henrique Barboza
2026-06-23  9:58         ` Peter Maydell
2026-06-23 11:38           ` Daniel Henrique Barboza
2026-06-23 16:10             ` Pierrick Bouvier
2026-06-23 19:01               ` Daniel Henrique Barboza
2026-06-23 19:16                 ` Pierrick Bouvier
2026-06-26  1:41                   ` Alistair Francis
2026-06-26 15:28                     ` Pierrick Bouvier [this message]
2026-06-27  3:31                   ` Konstantin Ryabitsev
2026-06-27  7:00                     ` Pierrick Bouvier
2026-06-26  1:37             ` Alistair Francis
2026-06-26 15:25               ` Pierrick Bouvier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=feb50737-7897-4881-95f1-cdf1725bb7a2@oss.qualcomm.com \
    --to=pierrick.bouvier@oss.qualcomm.com \
    --cc=alistair.francis@wdc.com \
    --cc=alistair23@gmail.com \
    --cc=chao.liu.zevorn@gmail.com \
    --cc=daniel.barboza@oss.qualcomm.com \
    --cc=liwei1518@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@oss.qualcomm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=zhiwei_liu@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.