Re: [PATCH v2 2/3] hw/riscv/riscv-iommu.c: fault when !PTE_U and no priv access

On 7/11/26 11:30, Michael Tokarev wrote:
> 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've dropped this patch from 10.0.x (lts) series for now,
while picking up other patches - in order to go with this
at least somewhere.  But I really don't know what to do
with all this :)  Lemme sleep on that for some time :)

Thanks,

/mjt