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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.