Michael Tokarev <[email protected]> writes: > On 7/1/26 15:11, Daniel Henrique Barboza wrote: >> All IOMMU accesses are assumed to be user mode unless told otherwise, >> i.e. we have a process_id. In case we have a non-user mode leaf PTE >> (PTE_U isn't set) and we are running in user mode, we need to throw a >> fault. >> >> This also reflects on qos-riscv-iommu tests: the tests always run in >> user mode so our PTEs must have PTE_U (bit 0x10) set. >> >> Fixes: 0c54acb8243d ("hw/riscv: add RISC-V IOMMU base emulation") >> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3553 >> Signed-off-by: Daniel Henrique Barboza <[email protected]> >> Reviewed-by: Nutty Liu <[email protected]> >> Reviewed-by: Chao Liu <[email protected]> > > ... >> --- a/tests/qtest/libqos/qos-riscv-iommu.h >> +++ b/tests/qtest/libqos/qos-riscv-iommu.h >> @@ -54,8 +54,8 @@ >> * PTE masks for RISC-V IOMMU page tables. >> * Values match PTE_V, PTE_R, PTE_W, PTE_A, PTE_D in >> target/riscv/cpu_bits.h >> */ >> -#define QRIOMMU_NON_LEAF_PTE_MASK 0x001 /* PTE_V */ >> -#define QRIOMMU_LEAF_PTE_RW_MASK 0x0c7 /* V|R|W|A|D */ >> +#define QRIOMMU_NON_LEAF_PTE_MASK 0x011 /* PTE_V | PTE_U */ >> +#define QRIOMMU_LEAF_PTE_RW_MASK 0x0d7 /* V | R | W | A | D | PTE_U */ >> #define QRIOMMU_PTE_PPN_MASK 0x003ffffffffffc00ull >> >> /* Address-space base offset for test tables */ > > So, this patch adds a fix for the testing bits. > > When trying to pick this one up for 10.0.x (LTS) series, I've another doubt. > This testing fix fixes v10.2.0-1299-g9d8ffbfc1d3 "tests/qtest/libqos: Add > RISC-V > IOMMU helper library". Quite some tests were added based on that library. > It doesn't exist in 10.0.x, and neither is v10.2.0-656-g489812e32df > "tests/qtest/libqos: Add SMMUv3 helper library". > Sure I can drop the parts of this change which touches the tests. But this > means we don't have tests to cover the issues being fixed, and I don't know > if the result of these fixes actually works or not. > > Should we pick up some testing bits in this area for 10.0.x (especially > 9d8ffbfc1d3 and some subsequent commits which use this library)? Or just > ignore all riscv iommu patches in there? >
I worry picking tests for stable could become a development task, requiring new code to make tests suitable for an older codebase/test codebase. The test frameworks and their supporting infrastructure (such as the riscv iommu lib) don't ensure a stable abi so that a test from one version will run without issues when backported to an earlier version. Also, the tests are usually not structured in a way that allow us to pick just the part that tests the code fixes being backported, so we risk trying to test in an earlier version features that don't even exist at that point. There could also exist a complex graph of what's broken vs. what is being tested (e.g. broken in v10, fixed partially in v11, then test needs change, then fixed properly in v12, test needs change again, etc). > Thanks, > > /mjt
