On 2025/7/30 19:23, Andrew Cooper wrote:
> On 28/07/2025 6:03 am, Jiqian Chen wrote:
>> vpci_remove_register() only supports removing a register in a time,
>> but the follow-on changes need to remove all registers within a range.
>> So, refactor it to support removing all registers in a given region.
>>
>> And it is no issue to remove a non exist register, so remove the
>> __must_check prefix.
>>
>> Signed-off-by: Jiqian Chen <jiqian.c...@amd.com>
>> Reviewed-by: Roger Pau Monné <roger....@citrix.com>
>
> Bisection says this causes an assertion failure in the unit test.
>
> Running /usr/lib/xen/tests/test_vpci
> Assertion failed: vpci_remove_registers(test_pdev.vpci, 16, 2) (main.c:
> main: 407)
> FAILED: /usr/lib/xen/tests/test_vpci
Thanks Andrew.
This is because new function vpci_remove_registers() removes all registers
inside (offset, offset + size) and returns failure when overlap happens.
For tools/tests/vpci/main.c, there are layout:
*
* 32 24 16 8 0
* +------+------+------+------+
* | r0 | 0
* +------+------+------+------+
* | r7 | r6 | r5 |//////| 4
* +------+------+------+------|
* |///////////////////////////| 8
* +-------------+-------------+
* |/////////////| r12 | 12
* +------+------+------+------+
* |r16[3]|r16[2]|r16[1]|r16[0]| 16
* +------+------+------+------+
* | r20[1] | r20[0] | 20
* +-------------+-------------|
* | r24 | 24
* +------+------+------+------+
* |//////| r30 |//////| r28 | 28
* +------+------+------+------+
* |rsvdz |rsvdp | rw1c | ro | 32
* +------+------+------+------+
*
As for the last three test cases:
VPCI_REMOVE_INVALID_REG(20, 1);
This can success as this overlap with r20[0]
VPCI_REMOVE_INVALID_REG(16, 2);
VPCI_REMOVE_INVALID_REG(30, 2);
These two fail as there are r16[0], r16[1] and r30 inside them, so remove
success and test cases fail.
So, I think we need to change these two test cases to match the new function's
logic, like:
VPCI_REMOVE_INVALID_REG(0, 2);
VPCI_REMOVE_INVALID_REG(2, 2);
Or delete them directly.
Hi Roger, what's your opinion?
>
> Full logs, not that they're of any more help:
>
> https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/1956817587
>
> ~Andrew
--
Best regards,
Jiqian Chen.
