| Message ID | 1432511251-22515-9-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On 05/24/2015 04:47 PM, Aurelien Jarno wrote: > Cc: Alexander Graf <agraf@suse.de> > Cc: Richard Henderson <rth@twiddle.net> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> Sadly, implementing this breaks current kernels. I did this about two years ago and havn't figured out what to do about it. The silly code in head.S doesn't check for just the facilities that it needs, it checks for all facilities that specific processors provide. If we don't e.g. pretend that we have HFP, despite the fact that no one uses it, the kernel won't boot. r~
On 26.05.15 01:08, Richard Henderson wrote: > On 05/24/2015 04:47 PM, Aurelien Jarno wrote: >> Cc: Alexander Graf <agraf@suse.de> >> Cc: Richard Henderson <rth@twiddle.net> >> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > Sadly, implementing this breaks current kernels. > I did this about two years ago and havn't figured > out what to do about it. > > The silly code in head.S doesn't check for just the > facilities that it needs, it checks for all facilities > that specific processors provide. > > If we don't e.g. pretend that we have HFP, despite the > fact that no one uses it, the kernel won't boot. It depends on how the kernel is configured too. You can select the minimum CPU type it can run on. But in most distros it's set to something way beyond our current emulation scope. But again, the real solution to this would be to have -cpu available and have TCG available feature masks possible to get outed-out. Alex
On 2015-05-25 16:08, Richard Henderson wrote: > On 05/24/2015 04:47 PM, Aurelien Jarno wrote: > >Cc: Alexander Graf <agraf@suse.de> > >Cc: Richard Henderson <rth@twiddle.net> > >Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > > Sadly, implementing this breaks current kernels. > I did this about two years ago and havn't figured > out what to do about it. > > The silly code in head.S doesn't check for just the > facilities that it needs, it checks for all facilities > that specific processors provide. > > If we don't e.g. pretend that we have HFP, despite the > fact that no one uses it, the kernel won't boot. How does it breaks? This patch only enable bits corresponding to facility that we fully implement (that's a reason why the patchset doesn't enable LD facility for example). So it should not be a problem, at least I have been able to boot kernels successfully.
On 05/25/2015 11:03 PM, Aurelien Jarno wrote: > On 2015-05-25 16:08, Richard Henderson wrote: >> On 05/24/2015 04:47 PM, Aurelien Jarno wrote: >>> Cc: Alexander Graf <agraf@suse.de> >>> Cc: Richard Henderson <rth@twiddle.net> >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> >> >> Sadly, implementing this breaks current kernels. >> I did this about two years ago and havn't figured >> out what to do about it. >> >> The silly code in head.S doesn't check for just the >> facilities that it needs, it checks for all facilities >> that specific processors provide. >> >> If we don't e.g. pretend that we have HFP, despite the >> fact that no one uses it, the kernel won't boot. > > How does it breaks? This patch only enable bits corresponding to > facility that we fully implement (that's a reason why the patchset > doesn't enable LD facility for example). So it should not be a problem, > at least I have been able to boot kernels successfully. > Unless the facilities provided are a strict superset of those required by the kernel, it aborts the boot. Extremely early in the process. As Alex said, it's possible to compile a kernel without this, but none of the vendor-built kernels do so. Which raises the bar to what a user needs to do to install. See linux/arch/s390/kernel/head.S, # List of facilities that are required. If not all facilities are present # the kernel will crash. Format is number of facility words with bits set, # followed by the facility words. #if defined(CONFIG_MARCH_Z13) .long 3, 0xc100eff2, 0xf46ce800, 0x00400000 #elif defined(CONFIG_MARCH_ZEC12) .long 3, 0xc100eff2, 0xf46ce800, 0x00400000 #elif defined(CONFIG_MARCH_Z196) .long 2, 0xc100eff2, 0xf46c0000 #elif defined(CONFIG_MARCH_Z10) .long 2, 0xc100eff2, 0xf0680000 #elif defined(CONFIG_MARCH_Z9_109) .long 1, 0xc100efc2 #elif defined(CONFIG_MARCH_Z990) .long 1, 0xc0002000 #elif defined(CONFIG_MARCH_Z900) .long 1, 0xc0000000 #endif r~
On 2015-05-26 09:00, Richard Henderson wrote: > On 05/25/2015 11:03 PM, Aurelien Jarno wrote: > > On 2015-05-25 16:08, Richard Henderson wrote: > >> On 05/24/2015 04:47 PM, Aurelien Jarno wrote: > >>> Cc: Alexander Graf <agraf@suse.de> > >>> Cc: Richard Henderson <rth@twiddle.net> > >>> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > >> > >> Sadly, implementing this breaks current kernels. > >> I did this about two years ago and havn't figured > >> out what to do about it. > >> > >> The silly code in head.S doesn't check for just the > >> facilities that it needs, it checks for all facilities > >> that specific processors provide. > >> > >> If we don't e.g. pretend that we have HFP, despite the > >> fact that no one uses it, the kernel won't boot. > > > > How does it breaks? This patch only enable bits corresponding to > > facility that we fully implement (that's a reason why the patchset > > doesn't enable LD facility for example). So it should not be a problem, > > at least I have been able to boot kernels successfully. > > > > Unless the facilities provided are a strict superset of those required by the > kernel, it aborts the boot. Extremely early in the process. As Alex said, > it's possible to compile a kernel without this, but none of the vendor-built > kernels do so. Which raises the bar to what a user needs to do to install. > > See linux/arch/s390/kernel/head.S, Ok, I understand now. That said I don't see how implementing STFLE will break that. I think it will actually improve things by enabling more facilities and thus making the kernel happier (but maybe not enough). > # List of facilities that are required. If not all facilities are present > # the kernel will crash. Format is number of facility words with bits set, > # followed by the facility words. > > #if defined(CONFIG_MARCH_Z13) > .long 3, 0xc100eff2, 0xf46ce800, 0x00400000 > #elif defined(CONFIG_MARCH_ZEC12) > .long 3, 0xc100eff2, 0xf46ce800, 0x00400000 > #elif defined(CONFIG_MARCH_Z196) > .long 2, 0xc100eff2, 0xf46c0000 > #elif defined(CONFIG_MARCH_Z10) > .long 2, 0xc100eff2, 0xf0680000 > #elif defined(CONFIG_MARCH_Z9_109) > .long 1, 0xc100efc2 This one looks a bit complicated to reach. Basically we need to implement STFLE, MSA, HFP and ETF2, 3 and 3e. But I guess that's something doable on the medium term > #elif defined(CONFIG_MARCH_Z990) > .long 1, 0xc0002000 For this one we only miss one instruction in the LD facility. I have it on my TODO list, though it might take a few weeks until it goes to the top. > #elif defined(CONFIG_MARCH_Z900) > .long 1, 0xc0000000 This corresponds to the current value we provide. CONFIG_MARCH_Z900 also correspond to the configuration of the Debian kernel, that's why I am able to boot kernels.
On 05/26/2015 03:03 PM, Aurelien Jarno wrote: > Ok, I understand now. That said I don't see how implementing STFLE will > break that. I think it will actually improve things by enabling more > facilities and thus making the kernel happier (but maybe not enough). Somewhat amusingly, by not implementing STFLE we bypass this check. >> #elif defined(CONFIG_MARCH_Z990) >> .long 1, 0xc0002000 > > For this one we only miss one instruction in the LD facility. I have it > on my TODO list, though it might take a few weeks until it goes to the > top. > Which one? >> #elif defined(CONFIG_MARCH_Z900) >> .long 1, 0xc0000000 > > This corresponds to the current value we provide. CONFIG_MARCH_Z900 also > correspond to the configuration of the Debian kernel, that's why I am > able to boot kernels. Ah, well that's something. Fedora defaults to z9-109, and I think SuSE does the same. r~
> Am 27.05.2015 um 16:31 schrieb Richard Henderson <rth@twiddle.net>: > >> On 05/26/2015 03:03 PM, Aurelien Jarno wrote: >> Ok, I understand now. That said I don't see how implementing STFLE will >> break that. I think it will actually improve things by enabling more >> facilities and thus making the kernel happier (but maybe not enough). > > Somewhat amusingly, by not implementing STFLE we bypass this check. > > >>> #elif defined(CONFIG_MARCH_Z990) >>> .long 1, 0xc0002000 >> >> For this one we only miss one instruction in the LD facility. I have it >> on my TODO list, though it might take a few weeks until it goes to the >> top. > > Which one? > >>> #elif defined(CONFIG_MARCH_Z900) >>> .long 1, 0xc0000000 >> >> This corresponds to the current value we provide. CONFIG_MARCH_Z900 also >> correspond to the configuration of the Debian kernel, that's why I am >> able to boot kernels. > > Ah, well that's something. Fedora defaults to z9-109, and I think SuSE does > the same. Unfortunately SLE12 has its ALS on z196, so there's quite a bit more work necessary ;) Alex > > > r~
On 2015-05-27 07:31, Richard Henderson wrote: > On 05/26/2015 03:03 PM, Aurelien Jarno wrote: > > Ok, I understand now. That said I don't see how implementing STFLE will > > break that. I think it will actually improve things by enabling more > > facilities and thus making the kernel happier (but maybe not enough). > > Somewhat amusingly, by not implementing STFLE we bypass this check. Oh, I see the whole picture now. > >> #elif defined(CONFIG_MARCH_Z990) > >> .long 1, 0xc0002000 > > > > For this one we only miss one instruction in the LD facility. I have it > > on my TODO list, though it might take a few weeks until it goes to the > > top. > > > > Which one? CVBY, but anyway we don't implement CVB and CVBG either... > >> #elif defined(CONFIG_MARCH_Z900) > >> .long 1, 0xc0000000 > > > > This corresponds to the current value we provide. CONFIG_MARCH_Z900 also > > correspond to the configuration of the Debian kernel, that's why I am > > able to boot kernels. > > Ah, well that's something. Fedora defaults to z9-109, and I think SuSE does > the same. One strategy would be to enable the bit in STFLE whether the feature is fully implemented by TCG or not, using the values provided by the CPU model (IBM patches). After all we have plenty of non-implemented basic instructions (e.g. all the HFP ones). The risk is that the userland starts to use some optimized libraries due to that. On the other hand if the kernel is compiled with this option, chances are that the userland is built with the same instruction set. Having a quick look at big sets of missing instructions, it looks like we are mostly missing HFP (most are in the basic instructions), DFP, high-word, extended translations, and message-security-assist. high-word facility should be relatively easy to add, extended translation a bit more complex. We might want to use libdecnumber like on PPC fro DFP. It looks like the most problematic are therefore HFP (but that's already the case anyway) and message-security-assist. Then there are plenty of facilities with only a few instructions missing. They might be tricky to implement though (i.e. transactional memory). One other strategy would be to create a "any" CPU for linux-user and also offer it to the softmmu mode.
> Am 27.05.2015 um 17:57 schrieb Aurelien Jarno <aurelien@aurel32.net>: > >> On 2015-05-27 07:31, Richard Henderson wrote: >>> On 05/26/2015 03:03 PM, Aurelien Jarno wrote: >>> Ok, I understand now. That said I don't see how implementing STFLE will >>> break that. I think it will actually improve things by enabling more >>> facilities and thus making the kernel happier (but maybe not enough). >> >> Somewhat amusingly, by not implementing STFLE we bypass this check. > > Oh, I see the whole picture now. > >>>> #elif defined(CONFIG_MARCH_Z990) >>>> .long 1, 0xc0002000 >>> >>> For this one we only miss one instruction in the LD facility. I have it >>> on my TODO list, though it might take a few weeks until it goes to the >>> top. >> >> Which one? > > CVBY, but anyway we don't implement CVB and CVBG either... > > >>>> #elif defined(CONFIG_MARCH_Z900) >>>> .long 1, 0xc0000000 >>> >>> This corresponds to the current value we provide. CONFIG_MARCH_Z900 also >>> correspond to the configuration of the Debian kernel, that's why I am >>> able to boot kernels. >> >> Ah, well that's something. Fedora defaults to z9-109, and I think SuSE does >> the same. > > One strategy would be to enable the bit in STFLE whether the feature is > fully implemented by TCG or not, using the values provided by the CPU > model (IBM patches). So how about we add a "force" parameter to the -cpu command line option to suppress masking of the facility bits with what tcg implements? > After all we have plenty of non-implemented basic > instructions (e.g. all the HFP ones). The risk is that the userland > starts to use some optimized libraries due to that. On the other hand > if the kernel is compiled with this option, chances are that the > userland is built with the same instruction set. > > Having a quick look at big sets of missing instructions, it looks like > we are mostly missing HFP (most are in the basic instructions), DFP, > high-word, extended translations, and message-security-assist. high-word > facility should be relatively easy to add, extended translation a bit > more complex. We might want to use libdecnumber like on PPC fro DFP. It > looks like the most problematic are therefore HFP (but that's already > the case anyway) and message-security-assist. > > Then there are plenty of facilities with only a few instructions > missing. They might be tricky to implement though (i.e. transactional > memory). On ppc we just fail every transaction which seems to do the trick. Can we do the same for s390? > > One other strategy would be to create a "any" CPU for linux-user and > also offer it to the softmmu mode. I think for softmmu we should stick with real world models, either masked with tcg's implemented facilities or not. In fact, how about instead of masking, we just make -cpu fail on creation if it wants a facility that tcg doesn't implement yet? Then we can hint the user to -cpu ec12,force to make it work nevertheless Alex > > -- > Aurelien Jarno GPG: 4096R/1DDD8C9B > aurelien@aurel32.net http://www.aurel32.net
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h index 35bfdec..3110c1f 100644 --- a/target-s390x/cpu.h +++ b/target-s390x/cpu.h @@ -174,7 +174,7 @@ static const uint64_t facilities_dw[] = { (0ULL << 59) | /* b 4: IDTE selective clearing when segtab invalidated */ (0ULL << 58) | /* b 5: IDTE selective clearing when regtab invalidated */ (0ULL << 57) | /* b 6: ASN-and-LX-reuse facility */ - (0ULL << 56) | /* b 7: Store-facility-list-extended facility */ + (1ULL << 56) | /* b 7: Store-facility-list-extended facility */ (0ULL << 55) | /* b 8: Enhanced-DAT facility */ (0ULL << 54) | /* b 9: Sense-running-status facility */ (0ULL << 53) | /* b10: Conditional-SSKE facility */ diff --git a/target-s390x/helper.h b/target-s390x/helper.h index e6f2afb..2dc01a0 100644 --- a/target-s390x/helper.h +++ b/target-s390x/helper.h @@ -79,6 +79,7 @@ DEF_HELPER_FLAGS_5(calc_cc, TCG_CALL_NO_RWG_SE, i32, env, i32, i64, i64, i64) DEF_HELPER_FLAGS_2(sfpc, TCG_CALL_NO_RWG, void, env, i64) DEF_HELPER_FLAGS_2(sfas, TCG_CALL_NO_WG, void, env, i64) DEF_HELPER_FLAGS_1(popcnt, TCG_CALL_NO_RWG_SE, i64, i64) +DEF_HELPER_2(stfle, i32, env, i64) #ifndef CONFIG_USER_ONLY DEF_HELPER_3(servc, i32, env, i64, i64) diff --git a/target-s390x/insn-data.def b/target-s390x/insn-data.def index 72c3a2e..fd45730 100644 --- a/target-s390x/insn-data.def +++ b/target-s390x/insn-data.def @@ -827,6 +827,8 @@ /* STORE CPU TIMER */ C(0xb209, STPT, S, Z, la2, 0, new, m1_64, stpt, 0) /* STORE FACILITY LIST */ + C(0xb2b0, STFLE, S, SFLE, 0, a2, 0, 0, stfle, 0) +/* STORE FACILITY LIST */ C(0xb2b1, STFL, S, Z, 0, 0, 0, 0, stfl, 0) /* STORE PREFIX */ C(0xb211, STPX, S, Z, la2, 0, new, m1_32, stpx, 0) diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c index b375ab7..711f365 100644 --- a/target-s390x/misc_helper.c +++ b/target-s390x/misc_helper.c @@ -76,6 +76,25 @@ void HELPER(exception)(CPUS390XState *env, uint32_t excp) cpu_loop_exit(cs); } +/* Store faciliy list extended */ +uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t dest) +{ + int nf = sizeof(facilities_dw) / sizeof(facilities_dw[0]); + int rf = (env->regs[0] & 0xff) + 1; + int i; + + for (i = 0; i < MIN(nf, rf); i++) { + cpu_stq_data(env, dest, facilities_dw[i]); + dest += 8; + } + + if (rf > nf) { + env->regs[0] = (env->regs[0] & ~0xff) | (nf - 1); + } + + return (rf < nf) ? 3 : 0; +} + #ifndef CONFIG_USER_ONLY void program_interrupt(CPUS390XState *env, uint32_t code, int ilen) diff --git a/target-s390x/translate.c b/target-s390x/translate.c index 542da53..78b8cdc 100644 --- a/target-s390x/translate.c +++ b/target-s390x/translate.c @@ -3382,6 +3382,13 @@ static ExitStatus op_stfl(DisasContext *s, DisasOps *o) return NO_EXIT; } +static ExitStatus op_stfle(DisasContext *s, DisasOps *o) +{ + gen_helper_stfle(cc_op, cpu_env, o->in2); + set_cc_static(s); + return NO_EXIT; +} + static ExitStatus op_stpt(DisasContext *s, DisasOps *o) { check_privileged(s);
Cc: Alexander Graf <agraf@suse.de> Cc: Richard Henderson <rth@twiddle.net> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- target-s390x/cpu.h | 2 +- target-s390x/helper.h | 1 + target-s390x/insn-data.def | 2 ++ target-s390x/misc_helper.c | 19 +++++++++++++++++++ target-s390x/translate.c | 7 +++++++ 5 files changed, 30 insertions(+), 1 deletion(-)