Re: [PATCH v2] s390x: Properly fetch the short psw on diag308 subc 0/1
> Am 12.11.2019 um 17:58 schrieb Cornelia Huck : > > On Mon, 11 Nov 2019 10:28:08 -0500 > Janosch Frank wrote: > >> We need to actually fetch the cpu mask and set it. As we invert the >> short psw indication in the mask, SIE will report a specification >> exception, if it wasn't present in the reset psw. >> >> Signed-off-by: Janosch Frank >> --- >> target/s390x/cpu.c | 12 ++-- >> target/s390x/cpu.h | 1 + >> 2 files changed, 11 insertions(+), 2 deletions(-) > > So, is this change -rc material, or should it go in during the next > release? I'm a bit confused here. IMHO, this is not urgent and can wait. > > [Also, does this need a change in the tcg code, or is that something > that should just be done eventually? Sorry, drowning a bit in mails > here...] We‘re missing many checks when loading/running a new PSW for TCG, not just this scenario. So this should be done at one point but is not urgent at all.
Re: [PATCH v2] s390x: Properly fetch the short psw on diag308 subc 0/1
On Mon, 11 Nov 2019 10:28:08 -0500 Janosch Frank wrote: > We need to actually fetch the cpu mask and set it. As we invert the > short psw indication in the mask, SIE will report a specification > exception, if it wasn't present in the reset psw. > > Signed-off-by: Janosch Frank > --- > target/s390x/cpu.c | 12 ++-- > target/s390x/cpu.h | 1 + > 2 files changed, 11 insertions(+), 2 deletions(-) So, is this change -rc material, or should it go in during the next release? I'm a bit confused here. [Also, does this need a change in the tcg code, or is that something that should just be done eventually? Sorry, drowning a bit in mails here...]
Re: [PATCH v2] s390x: Properly fetch the short psw on diag308 subc 0/1
Patchew URL: https://patchew.org/QEMU/2019152808.13371-1-fran...@linux.ibm.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH v2] s390x: Properly fetch the short psw on diag308 subc 0/1 Type: series Message-id: 2019152808.13371-1-fran...@linux.ibm.com === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 74d60dc s390x: Properly fetch the short psw on diag308 subc 0/1 === OUTPUT BEGIN === ERROR: code indent should never use tabs #46: FILE: target/s390x/cpu.h:268: +#define PSW_MASK_SHORTPSW^I0x0008ULL$ total: 1 errors, 0 warnings, 25 lines checked Commit 74d60dc486a0 (s390x: Properly fetch the short psw on diag308 subc 0/1) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/2019152808.13371-1-fran...@linux.ibm.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2] s390x: Properly fetch the short psw on diag308 subc 0/1
> Am 11.11.2019 um 16:28 schrieb Janosch Frank :
>
> We need to actually fetch the cpu mask and set it. As we invert the
> short psw indication in the mask, SIE will report a specification
> exception, if it wasn't present in the reset psw.
>
> Signed-off-by: Janosch Frank
> ---
> target/s390x/cpu.c | 12 ++--
> target/s390x/cpu.h | 1 +
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 736a7903e2..40aa42e092 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -76,8 +76,16 @@ static bool s390_cpu_has_work(CPUState *cs)
> static void s390_cpu_load_normal(CPUState *s)
> {
> S390CPU *cpu = S390_CPU(s);
> -cpu->env.psw.addr = ldl_phys(s->as, 4) & PSW_MASK_ESA_ADDR;
> -cpu->env.psw.mask = PSW_MASK_32 | PSW_MASK_64;
> +uint64_t spsw = ldq_phys(s->as, 0);
> +
> +cpu->env.psw.mask = spsw & 0x8000UL;
ULL
> +/*
> + * Invert short psw indication, so SIE will report a specification
> + * exception if it was not set.
> + */
It would be interesting to know how the PSW mask in the PGM old PSW looks like
on LPAR. IOW, „you forgot to set the short indication, here is an exception.
see, the short indication is set now.“ Sounds weird, most probably nobody cares.
> +cpu->env.psw.mask ^= PSW_MASK_SHORTPSW;
> +cpu->env.psw.addr = spsw & 0x7fffUL;
Eventually also ULL
> +
> s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
> }
> #endif
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 947553386f..2c687185f1 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -261,6 +261,7 @@ extern const struct VMStateDescription vmstate_s390_cpu;
> #define PSW_MASK_EXT0x0100ULL
> #define PSW_MASK_KEY0x00F0ULL
> #define PSW_SHIFT_KEY 52
> +#define PSW_MASK_SHORTPSW0x0008ULL
> #define PSW_MASK_MCHECK 0x0004ULL
> #define PSW_MASK_WAIT 0x0002ULL
> #define PSW_MASK_PSTATE 0x0001ULL
> --
> 2.20.1
>
Looks good to me
Reviewed-by: David Hildenbrand
[PATCH v2] s390x: Properly fetch the short psw on diag308 subc 0/1
We need to actually fetch the cpu mask and set it. As we invert the
short psw indication in the mask, SIE will report a specification
exception, if it wasn't present in the reset psw.
Signed-off-by: Janosch Frank
---
target/s390x/cpu.c | 12 ++--
target/s390x/cpu.h | 1 +
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 736a7903e2..40aa42e092 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -76,8 +76,16 @@ static bool s390_cpu_has_work(CPUState *cs)
static void s390_cpu_load_normal(CPUState *s)
{
S390CPU *cpu = S390_CPU(s);
-cpu->env.psw.addr = ldl_phys(s->as, 4) & PSW_MASK_ESA_ADDR;
-cpu->env.psw.mask = PSW_MASK_32 | PSW_MASK_64;
+uint64_t spsw = ldq_phys(s->as, 0);
+
+cpu->env.psw.mask = spsw & 0x8000UL;
+/*
+ * Invert short psw indication, so SIE will report a specification
+ * exception if it was not set.
+ */
+cpu->env.psw.mask ^= PSW_MASK_SHORTPSW;
+cpu->env.psw.addr = spsw & 0x7fffUL;
+
s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
}
#endif
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 947553386f..2c687185f1 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -261,6 +261,7 @@ extern const struct VMStateDescription vmstate_s390_cpu;
#define PSW_MASK_EXT0x0100ULL
#define PSW_MASK_KEY0x00F0ULL
#define PSW_SHIFT_KEY 52
+#define PSW_MASK_SHORTPSW 0x0008ULL
#define PSW_MASK_MCHECK 0x0004ULL
#define PSW_MASK_WAIT 0x0002ULL
#define PSW_MASK_PSTATE 0x0001ULL
--
2.20.1
