[PATCH v3 4/4] KVM: selftests: aarch64: Test VGIC ITS tables save/restore

[Jing Zhang]

Hi Marc,


On Wed, Nov 6, 2024 at 5:27 AM Marc Zyngier <maz at kernel.org> wrote:
>
> [Adding Eric to the list, since he worked a lot on the save/restore code]
>
> On Wed, 06 Nov 2024 08:30:35 +0000,
> Jing Zhang <jingzhangos at google.com> wrote:
> >
> > Add a selftest to verify the correctness of the VGIC ITS mappings after
> > the save/restore operations (KVM_DEV_ARM_ITS_SAVE_TABLES /
> > KVM_DEV_ARM_ITS_RESTORE_TABLES).
>
> What are you checking? The saved data? The restored data?
>
> > Also calculate the time spending on save/restore operations.
>
> Is that really relevant? I don't think performance matters at this
> stage, if we can't even have reliable data.

Right. It doesn't matter. Will remove this line from the message.
>
> > This test uses some corner cases to capture the save/restore bugs. It
>
> Which corner cases?

Will add details.
>
> > will be used to verify the future incoming changes for the VGIC ITS
> > tables save/restore.
> >
> > To capture the "Invalid argument (-22)" error, run the test without any
> > option. To capture the wrong/lost mappings, run the test with '-s'
> > option.
> > Since the VGIC ITS save/restore bug is caused by orphaned DTE/ITE
> > entries, if we run the test with '-c' option whih clears the tables
> > before the save operation, the test will complete successfully.
>
> I'm sorry, but this description is meaningless, as you need to know
> what is the bug that has been fixed.

Will add bug description here too as in the cover letter.
>
> Also, how is someone supposed to run this thing? Without options? With
> options? With any combination of options?
>
> From what I understand, the various options are designed to help
> debugging a broken vgic implementation. So please document what the
> options do rather than an bug that is supposed to be already fixed.

Will do.
>
> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

Thanks,
Jing