| Message ID | 20220405075225.15903-3-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Cleanup and maintenance 4 | expand |
On 05/04/2022 09.52, Janosch Frank wrote: > Other hypervisors might implement it and therefore not send a > specification exception. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/diag308.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/s390x/diag308.c b/s390x/diag308.c > index c9d6c499..9614f9a9 100644 > --- a/s390x/diag308.c > +++ b/s390x/diag308.c > @@ -8,6 +8,7 @@ > #include <libcflat.h> > #include <asm/asm-offsets.h> > #include <asm/interrupt.h> > +#include <hardware.h> > > /* The diagnose calls should be blocked in problem state */ > static void test_priv(void) > @@ -75,7 +76,7 @@ static void test_subcode6(void) > /* Unsupported subcodes should generate a specification exception */ > static void test_unsupported_subcode(void) > { > - int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 }; > + int subcodes[] = { 0x101, 0xffff, 0x10001, -1 }; > int idx; > > for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) { > @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void) > check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > report_prefix_pop(); > } > + > + /* > + * Subcode 2 is not available under QEMU but might be on other > + * hypervisors. > + */ > + if (detect_host() != HOST_IS_TCG && detect_host() != HOST_IS_KVM) { Shouldn't this be rather the other way round instead? if (detect_host() == HOST_IS_TCG || detect_host() == HOST_IS_KVM) ? ... anyway, since you already used a similar if-clause in your first patch, it might make sense to add a helper function a la host_is_qemu() to check whether we're running on QEMU or not. > + report_prefix_pushf("0x%04x", 2); report_prefix_pushf("0x02") ? > + expect_pgm_int(); > + asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2)); > + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > + report_prefix_pop(); > + } > } Thomas
On 4/5/22 11:18, Thomas Huth wrote: > On 05/04/2022 09.52, Janosch Frank wrote: >> Other hypervisors might implement it and therefore not send a >> specification exception. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> s390x/diag308.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/s390x/diag308.c b/s390x/diag308.c >> index c9d6c499..9614f9a9 100644 >> --- a/s390x/diag308.c >> +++ b/s390x/diag308.c >> @@ -8,6 +8,7 @@ >> #include <libcflat.h> >> #include <asm/asm-offsets.h> >> #include <asm/interrupt.h> >> +#include <hardware.h> >> >> /* The diagnose calls should be blocked in problem state */ >> static void test_priv(void) >> @@ -75,7 +76,7 @@ static void test_subcode6(void) >> /* Unsupported subcodes should generate a specification exception */ >> static void test_unsupported_subcode(void) >> { >> - int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 }; >> + int subcodes[] = { 0x101, 0xffff, 0x10001, -1 }; >> int idx; >> >> for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) { >> @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void) >> check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> report_prefix_pop(); >> } >> + >> + /* >> + * Subcode 2 is not available under QEMU but might be on other >> + * hypervisors. >> + */ >> + if (detect_host() != HOST_IS_TCG && detect_host() != HOST_IS_KVM) { > > Shouldn't this be rather the other way round instead? > > if (detect_host() == HOST_IS_TCG || detect_host() == HOST_IS_KVM) > > ? The css if checks if we are under QEMU, this one checks if we're not under QEMU. > > ... anyway, since you already used a similar if-clause in your first patch, > it might make sense to add a helper function a la host_is_qemu() to check > whether we're running on QEMU or not. Will do > >> + report_prefix_pushf("0x%04x", 2); > > report_prefix_pushf("0x02") ? > >> + expect_pgm_int(); >> + asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2)); >> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >> + report_prefix_pop(); >> + } >> } > > Thomas >
On 05/04/2022 11.33, Janosch Frank wrote: > On 4/5/22 11:18, Thomas Huth wrote: >> On 05/04/2022 09.52, Janosch Frank wrote: >>> Other hypervisors might implement it and therefore not send a >>> specification exception. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> s390x/diag308.c | 15 ++++++++++++++- >>> 1 file changed, 14 insertions(+), 1 deletion(-) >>> >>> diff --git a/s390x/diag308.c b/s390x/diag308.c >>> index c9d6c499..9614f9a9 100644 >>> --- a/s390x/diag308.c >>> +++ b/s390x/diag308.c >>> @@ -8,6 +8,7 @@ >>> #include <libcflat.h> >>> #include <asm/asm-offsets.h> >>> #include <asm/interrupt.h> >>> +#include <hardware.h> >>> /* The diagnose calls should be blocked in problem state */ >>> static void test_priv(void) >>> @@ -75,7 +76,7 @@ static void test_subcode6(void) >>> /* Unsupported subcodes should generate a specification exception */ >>> static void test_unsupported_subcode(void) >>> { >>> - int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 }; >>> + int subcodes[] = { 0x101, 0xffff, 0x10001, -1 }; >>> int idx; >>> for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) { >>> @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void) >>> check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >>> report_prefix_pop(); >>> } >>> + >>> + /* >>> + * Subcode 2 is not available under QEMU but might be on other >>> + * hypervisors. >>> + */ >>> + if (detect_host() != HOST_IS_TCG && detect_host() != HOST_IS_KVM) { >> >> Shouldn't this be rather the other way round instead? >> >> if (detect_host() == HOST_IS_TCG || detect_host() == HOST_IS_KVM) >> >> ? > > The css if checks if we are under QEMU, this one checks if we're not under > QEMU. but ... >> >>> + expect_pgm_int(); >>> + asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2)); >>> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); ... don't we want to check here whether the diag causes a spec exception if we are *under* QEMU here? /me feels confused now. Thomas
On 4/5/22 11:50, Thomas Huth wrote: > On 05/04/2022 11.33, Janosch Frank wrote: >> On 4/5/22 11:18, Thomas Huth wrote: >>> On 05/04/2022 09.52, Janosch Frank wrote: >>>> Other hypervisors might implement it and therefore not send a >>>> specification exception. >>>> >>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>>> --- >>>> s390x/diag308.c | 15 ++++++++++++++- >>>> 1 file changed, 14 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/s390x/diag308.c b/s390x/diag308.c >>>> index c9d6c499..9614f9a9 100644 >>>> --- a/s390x/diag308.c >>>> +++ b/s390x/diag308.c >>>> @@ -8,6 +8,7 @@ >>>> #include <libcflat.h> >>>> #include <asm/asm-offsets.h> >>>> #include <asm/interrupt.h> >>>> +#include <hardware.h> >>>> /* The diagnose calls should be blocked in problem state */ >>>> static void test_priv(void) >>>> @@ -75,7 +76,7 @@ static void test_subcode6(void) >>>> /* Unsupported subcodes should generate a specification exception */ >>>> static void test_unsupported_subcode(void) >>>> { >>>> - int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 }; >>>> + int subcodes[] = { 0x101, 0xffff, 0x10001, -1 }; >>>> int idx; >>>> for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) { >>>> @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void) >>>> check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); >>>> report_prefix_pop(); >>>> } >>>> + >>>> + /* >>>> + * Subcode 2 is not available under QEMU but might be on other >>>> + * hypervisors. >>>> + */ >>>> + if (detect_host() != HOST_IS_TCG && detect_host() != HOST_IS_KVM) { >>> >>> Shouldn't this be rather the other way round instead? >>> >>> if (detect_host() == HOST_IS_TCG || detect_host() == HOST_IS_KVM) >>> >>> ? >> >> The css if checks if we are under QEMU, this one checks if we're not under >> QEMU. > > but ... > >>> >>>> + expect_pgm_int(); >>>> + asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2)); >>>> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); > > ... don't we want to check here whether the diag causes a spec exception if > we are *under* QEMU here? > /me feels confused now. Seems like it's me who's confused today Anyway, I'll clean that up. > > Thomas >
diff --git a/s390x/diag308.c b/s390x/diag308.c index c9d6c499..9614f9a9 100644 --- a/s390x/diag308.c +++ b/s390x/diag308.c @@ -8,6 +8,7 @@ #include <libcflat.h> #include <asm/asm-offsets.h> #include <asm/interrupt.h> +#include <hardware.h> /* The diagnose calls should be blocked in problem state */ static void test_priv(void) @@ -75,7 +76,7 @@ static void test_subcode6(void) /* Unsupported subcodes should generate a specification exception */ static void test_unsupported_subcode(void) { - int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 }; + int subcodes[] = { 0x101, 0xffff, 0x10001, -1 }; int idx; for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) { @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void) check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); report_prefix_pop(); } + + /* + * Subcode 2 is not available under QEMU but might be on other + * hypervisors. + */ + if (detect_host() != HOST_IS_TCG && detect_host() != HOST_IS_KVM) { + report_prefix_pushf("0x%04x", 2); + expect_pgm_int(); + asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2)); + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION); + report_prefix_pop(); + } } static struct {
Other hypervisors might implement it and therefore not send a specification exception. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- s390x/diag308.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)