Re: [PATCH] hw/riscv/riscv-iommu.c: set ftype and iova in riscv_iommu_ctx()
Akihiko Odaki <[email protected]>
| Newsgroups | org.nongnu.qemu-riscv,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
Hi,
First, thank you for your work maintaining the stable series.
On 2026/07/11 16:47, Michael Tokarev wrote:
> On 7/1/26 12:22, Daniel Henrique Barboza wrote:
>> We're hardcoding faulting type as READ, where it could very well be a
>> write access, and we're not recording the faulting addr/iova.
>>
>> A note was added in the fault_type logic because I wasn't able to
>> trivially handle a probable code repeitition it in this same patch.
>> Something to do in a later date.
>>
>> Fixes: 0c54acb8243d ("hw/riscv: add RISC-V IOMMU base emulation")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3564
>> Signed-off-by: Daniel Henrique Barboza <[email protected]>
>> ---
>> hw/riscv/riscv-iommu.c | 29 +++++++++++++++++++++++------
>> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> For the stable qemu series (currently 10.0.x-LTS and 11.0.x), in order
> to pick this commit, I also picked v11.0.0-1212-gca7bac51e04
> "hw/riscv/riscv-iommu: Avoid caching PCI device IDs" by Chengbo Gao
> (Cc'ed) - because it touched the same code and there's a merge conflict.
> I can edit this patch ("set ftype and iova in riscv_iommu_ctx()") instead,
> but it felt that ca7bac51e04 is also a good pick for the stable series.
>
> Additionally for 10.0.x-LTS series, I also picked 2 more commits:
>
> v10.2.0-323-g15406cc593a "hw/riscv: riscv-iommu: Don't look up DDT cache
> in Off and Bare modes" by Frank Chang (Cc'ed), which avoids a merge
> conflict and also looks like a (maybe missing) bugfix.
>
> v10.2.0-312-gc497ef6739c "hw/riscv/riscv-iommu: Fix MemoryRegion owner"
> by Akihiko Odaki, which I spotted when looking for the history of
> changes, and which looked like an obvious bugfix.
This particular change is somewhat orthogonal to the broader issue you
describe with RISC-V patches. I found it through a codebase-wide search
for incorrect MemoryRegion ownership, rather than while following the
evolution of the RISC-V code. The same search found a similar issue
elsewhere, which you have already picked up:
https://lore.kernel.org/qemu-devel/[email protected]/
("[Stable-10.0.7 18/81] hw/nvram/ds1225y: Fix nvram MemoryRegion owner")
Unfortunately, there are several recurring patterns of MemoryRegion
misuse, and in some cases I am not entirely sure what the correct
ownership should be. In any case, this is separate from the concerns
around fast-moving RISC-V development.
This particular change is trivial; it corrects semantically incorrect
code, but I am not sure whether the bug has any user-visible effect.
Regards,
Akihiko Odaki
>
> Hopefully this is okay. Unfortunately even the simpler changes brings
> up quite a lot of missing fixes just by a chance - this means that we
> either not doing a good job keeping stable series up to date (by marking
> commits fixing stuff for-stable), or we should not try fixing stuff in
> previous series for a too-fast-moving target.
>
> I'm sorry I'm bringing up this more general question in a very narrow
> context of this particular fix (which might not even be important
> enough). Maybe I should discuss this separately. Overall, I'm
> having some.. issues with riscv pullreqs in context of stable series,
> it's always more time-consuming to handle these and a bit more difficult
> to decide what is imortant and what can be skipped.
>
> Thanks,
>
> /mjt
>