From: Andrew Jones <drjones@redhat.com>
To: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: kvm@vger.kernel.org, pbonzini@redhat.com, frankja@linux.ibm.com,
david@redhat.com, cohuck@redhat.com, peterx@redhat.com,
thuth@redhat.com
Subject: Re: [PATCH 0/4] KVM: selftests: Various cleanups and fixes
Date: Tue, 10 Mar 2020 14:18:40 +0100 [thread overview]
Message-ID: <20200310131840.wvmgkli6ed3s366q@kamzik.brq.redhat.com> (raw)
In-Reply-To: <9d33b783-8911-d638-af34-d69eab3520ef@de.ibm.com>
On Tue, Mar 10, 2020 at 01:34:54PM +0100, Christian Borntraeger wrote:
> On 10.03.20 13:29, Christian Borntraeger wrote:
> > I get the following with your patches.
>
> And those errors have been there before.
> I will provide fixups for sync_regs_test.c and reset.c. So you patch set really has value.
Yeah, another thing I intended to say in the cover-letter, but forgot,
was that I hadn't tested on s390x, and that there was a chance
TEST_ASSERT warnings would be found.
Thanks for fixing them!
drew
>
>
> >
> >
> > In file included from s390x/sync_regs_test.c:21:
> > s390x/sync_regs_test.c: In function ‘compare_sregs’:
> > s390x/sync_regs_test.c:41:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 6 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
> > 41 | "Register " #reg \
> > | ^~~~~~~~~~~
> > 42 | " values did not match: 0x%llx, 0x%llx\n", \
> > 43 | left->reg, right->reg)
> > | ~~~~~~~~~~~~~~~~~~~~~~
> > 44 |
> > |
> > 45 | static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 46 | {
> > | ~
> > 47 | int i;
> > | ~~~~~~
> > 48 |
> > |
> > 49 | for (i = 0; i < 16; i++)
> > | ~~~~~~~~~~~~~~~~~~~~~~~~
> > 50 | REG_COMPARE(gprs[i]);
> > | ~~~~~~~~~~~~~~~~~~~~~
> > 51 | }
> > | ~
> > 52 |
> > |
> > 53 | static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 54 | {
> > | ~
> > 55 | int i;
> > | ~~~~~~
> > 56 |
> > |
> > 57 | for (i = 0; i < 16; i++)
> > | ~~~~~~~~~~~~~~~~~~~~~~~~
> > 58 | REG_COMPARE(acrs[i]);
> > | ~~~~~~~~~~~~~~~~~~~
> > | |
> > | __u32 {aka unsigned int}
> > include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
> > 46 | test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
> > | ^~~
> > s390x/sync_regs_test.c:58:3: note: in expansion of macro ‘REG_COMPARE’
> > 58 | REG_COMPARE(acrs[i]);
> > | ^~~~~~~~~~~
> > s390x/sync_regs_test.c:41:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 7 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
> > 41 | "Register " #reg \
> > | ^~~~~~~~~~~
> > 42 | " values did not match: 0x%llx, 0x%llx\n", \
> > 43 | left->reg, right->reg)
> > | ~~~~~~~~~~~
> > 44 |
> > |
> > 45 | static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 46 | {
> > | ~
> > 47 | int i;
> > | ~~~~~~
> > 48 |
> > |
> > 49 | for (i = 0; i < 16; i++)
> > | ~~~~~~~~~~~~~~~~~~~~~~~~
> > 50 | REG_COMPARE(gprs[i]);
> > | ~~~~~~~~~~~~~~~~~~~~~
> > 51 | }
> > | ~
> > 52 |
> > |
> > 53 | static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 54 | {
> > | ~
> > 55 | int i;
> > | ~~~~~~
> > 56 |
> > |
> > 57 | for (i = 0; i < 16; i++)
> > | ~~~~~~~~~~~~~~~~~~~~~~~~
> > 58 | REG_COMPARE(acrs[i]);
> > | ~~~~~~~~~~~~~~~~~~~
> > | |
> > | __u32 {aka unsigned int}
> > include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
> > 46 | test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
> > | ^~~
> > s390x/sync_regs_test.c:58:3: note: in expansion of macro ‘REG_COMPARE’
> > 58 | REG_COMPARE(acrs[i]);
> > | ^~~~~~~~~~~
> > s390x/sync_regs_test.c: In function ‘main’:
> > s390x/sync_regs_test.c:158:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 6 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
> > 158 | "acr0 sync regs value incorrect 0x%llx.",
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 159 | run->s.regs.acrs[0]);
> > | ~~~~~~~~~~~~~~~~~~~
> > | |
> > | __u32 {aka unsigned int}
> > include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
> > 46 | test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
> > | ^~~
> > s390x/sync_regs_test.c:158:44: note: format string is defined here
> > 158 | "acr0 sync regs value incorrect 0x%llx.",
> > | ~~~^
> > | |
> > | long long unsigned int
> > |
> >
> >
> >
> > On 10.03.20 12:58, Andrew Jones wrote:
> >> On Tue, Mar 10, 2020 at 10:45:43AM +0100, Christian Borntraeger wrote:
> >>> On 10.03.20 10:15, Andrew Jones wrote:
> >>>>
> >>>> Andrew Jones (4):
> >>>> fixup! selftests: KVM: SVM: Add vmcall test
> >>>> KVM: selftests: Share common API documentation
> >>>> KVM: selftests: Enable printf format warnings for TEST_ASSERT
> >>>> KVM: selftests: Use consistent message for test skipping
> >>>
> >>> This looks like a nice cleanup but this does not seem to apply
> >>> cleanly on kvm/master or linus/master. Which tree is this based on?
> >>
> >> This is based on kvm/queue. Sorry, I should have mentioned that in
> >> the cover letter.
> >>
> >> Thanks,
> >> drew
> >>
> >>>
> >>>>
> >>>> tools/testing/selftests/kvm/.gitignore | 5 +-
> >>>> .../selftests/kvm/demand_paging_test.c | 6 +-
> >>>> tools/testing/selftests/kvm/dirty_log_test.c | 3 +-
> >>>> .../testing/selftests/kvm/include/kvm_util.h | 100 ++++++++-
> >>>> .../testing/selftests/kvm/include/test_util.h | 5 +-
> >>>> .../selftests/kvm/lib/aarch64/processor.c | 17 --
> >>>> tools/testing/selftests/kvm/lib/assert.c | 6 +-
> >>>> tools/testing/selftests/kvm/lib/kvm_util.c | 10 +-
> >>>> .../selftests/kvm/lib/kvm_util_internal.h | 48 +++++
> >>>> .../selftests/kvm/lib/s390x/processor.c | 74 -------
> >>>> tools/testing/selftests/kvm/lib/test_util.c | 12 ++
> >>>> .../selftests/kvm/lib/x86_64/processor.c | 196 ++++--------------
> >>>> tools/testing/selftests/kvm/lib/x86_64/svm.c | 2 +-
> >>>> tools/testing/selftests/kvm/lib/x86_64/vmx.c | 2 +-
> >>>> tools/testing/selftests/kvm/s390x/memop.c | 2 +-
> >>>> .../selftests/kvm/s390x/sync_regs_test.c | 2 +-
> >>>> .../kvm/x86_64/cr4_cpuid_sync_test.c | 2 +-
> >>>> .../testing/selftests/kvm/x86_64/evmcs_test.c | 6 +-
> >>>> .../selftests/kvm/x86_64/hyperv_cpuid.c | 8 +-
> >>>> .../selftests/kvm/x86_64/mmio_warning_test.c | 4 +-
> >>>> .../selftests/kvm/x86_64/platform_info_test.c | 3 +-
> >>>> .../kvm/x86_64/set_memory_region_test.c | 3 +-
> >>>> .../testing/selftests/kvm/x86_64/state_test.c | 4 +-
> >>>> .../selftests/kvm/x86_64/svm_vmcall_test.c | 3 +-
> >>>> .../selftests/kvm/x86_64/sync_regs_test.c | 4 +-
> >>>> .../selftests/kvm/x86_64/vmx_dirty_log_test.c | 2 +-
> >>>> .../kvm/x86_64/vmx_set_nested_state_test.c | 4 +-
> >>>> .../selftests/kvm/x86_64/xss_msr_test.c | 2 +-
> >>>> 28 files changed, 243 insertions(+), 292 deletions(-)
> >>>>
> >>>
> >>
> >
>
next prev parent reply other threads:[~2020-03-10 13:18 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-10 9:15 [PATCH 0/4] KVM: selftests: Various cleanups and fixes Andrew Jones
2020-03-10 9:15 ` [PATCH 1/4] fixup! selftests: KVM: SVM: Add vmcall test Andrew Jones
2020-03-10 9:15 ` [PATCH 2/4] KVM: selftests: Share common API documentation Andrew Jones
2020-03-10 9:15 ` [PATCH 3/4] KVM: selftests: Enable printf format warnings for TEST_ASSERT Andrew Jones
2020-03-10 12:54 ` Christian Borntraeger
2020-03-10 9:15 ` [PATCH 4/4] KVM: selftests: Use consistent message for test skipping Andrew Jones
2020-03-10 9:45 ` [PATCH 0/4] KVM: selftests: Various cleanups and fixes Christian Borntraeger
2020-03-10 11:58 ` Andrew Jones
2020-03-10 12:29 ` Christian Borntraeger
2020-03-10 12:34 ` Christian Borntraeger
2020-03-10 13:18 ` Andrew Jones [this message]
2020-03-11 19:29 ` Peter Xu
2020-03-14 11:44 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200310131840.wvmgkli6ed3s366q@kamzik.brq.redhat.com \
--to=drjones@redhat.com \
--cc=borntraeger@de.ibm.com \
--cc=cohuck@redhat.com \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.