* [Qemu-trivial] [PATCH] PPC: MMU compatibility check. @ 2017-01-24 18:56 Valentin Plotkin 2017-01-24 20:32 ` [Qemu-trivial] [Qemu-ppc] " Thomas Huth 0 siblings, 1 reply; 4+ messages in thread From: Valentin Plotkin @ 2017-01-24 18:56 UTC (permalink / raw) To: qemu-trivial; +Cc: qemu-devel, qemu-ppc Hi everyone, I looked at the "qemu-system-ppc -nographic -cpu G2leGP3 -M ppce500" on the BiteSizedTasks page. The segfault was caused by machine initialization code which expected a certain MMU model, checked, so unused SPR were read, returning zeros. bamboo and virtex machines are affected as well, but it doesn't always cause segfault, usually running into unmapped memory and failing somewhat more nicely. I added the checks. It would be possible to add support for other MMU models, but I'm not sure if there is any point (would any guest OS support mutually exclusive CPU and machine)? --- hw/ppc/e500.c | 5 +++++ hw/ppc/ppc440_bamboo.c | 5 +++++ hw/ppc/virtex_ml507.c | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index cf8b122..cd352f6 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -631,6 +631,11 @@ static uint64_t mmubooke_initial_mapsize(CPUPPCState *env) static void mmubooke_create_initial_mapping(CPUPPCState *env) { + if(env->mmu_model!=POWERPC_MMU_BOOKE206){ + fprintf(stderr,"MMU model %i not supported by this machine.\n", env->mmu_model); + exit(-1); + } + ppcmas_tlb_t *tlb = booke206_get_tlbm(env, 1, 0, 0); hwaddr size; int ps; diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c index 5c535b1..61e7028 100644 --- a/hw/ppc/ppc440_bamboo.c +++ b/hw/ppc/ppc440_bamboo.c @@ -124,6 +124,11 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env, target_ulong va, hwaddr pa) { + if(env->mmu_model!=POWERPC_MMU_BOOKE){ + fprintf(stderr,"MMU model %i not supported by this machine.\n", env->mmu_model); + exit(-1); + } + ppcemb_tlb_t *tlb = &env->tlb.tlbe[0]; tlb->attr = 0; diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c index b97d966..4041ff3 100644 --- a/hw/ppc/virtex_ml507.c +++ b/hw/ppc/virtex_ml507.c @@ -69,6 +69,11 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env, target_ulong va, hwaddr pa) { + if(env->mmu_model!=POWERPC_MMU_BOOKE){ + fprintf(stderr,"MMU model %i not supported by this machine.\n", env->mmu_model); + exit(-1); + } + ppcemb_tlb_t *tlb = &env->tlb.tlbe[0]; tlb->attr = 0; -- 2.5.5 Valentin Plotkin caliborn@sdf.org SDF Public Access UNIX System - http://sdf.org ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-trivial] [Qemu-ppc] [PATCH] PPC: MMU compatibility check. 2017-01-24 18:56 [Qemu-trivial] [PATCH] PPC: MMU compatibility check Valentin Plotkin @ 2017-01-24 20:32 ` Thomas Huth 2017-01-24 21:41 ` Valentin Plotkin 0 siblings, 1 reply; 4+ messages in thread From: Thomas Huth @ 2017-01-24 20:32 UTC (permalink / raw) To: Valentin Plotkin, qemu-trivial; +Cc: qemu-ppc, qemu-devel On 24.01.2017 19:56, Valentin Plotkin wrote: > > Hi everyone, > > I looked at the "qemu-system-ppc -nographic -cpu G2leGP3 -M ppce500" on > the BiteSizedTasks page. The segfault was caused by machine > initialization code which expected a certain MMU model, checked, so > unused SPR were read, returning zeros. bamboo and virtex machines are > affected as well, but it doesn't always cause segfault, usually running > into unmapped memory and failing somewhat more nicely. > > I added the checks. It would be possible to add support for other MMU > models, but I'm not sure if there is any point (would any guest OS > support mutually exclusive CPU and machine)? Hi, great to have a fix for this crash! I don't think it make much sense to add support for other MMU models here, so the simple checks should be good enough. However, your new code obviously does not follow the QEMU coding style. Could you please run your patch through scripts/checkpatch.pl and fix all issues that it reports? And when you resubmit, please make sure to copy the maintainers on CC: as well (scripts/get_maintainers.pl is your friend here). Thanks, Thomas ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-trivial] [Qemu-ppc] [PATCH] PPC: MMU compatibility check. 2017-01-24 20:32 ` [Qemu-trivial] [Qemu-ppc] " Thomas Huth @ 2017-01-24 21:41 ` Valentin Plotkin 2017-01-25 8:12 ` Thomas Huth 0 siblings, 1 reply; 4+ messages in thread From: Valentin Plotkin @ 2017-01-24 21:41 UTC (permalink / raw) To: Thomas Huth Cc: Alexander Graf, David Gibson, Edgar E. Iglesias, qemu-ppc, qemu-devel, qemu-trivial On Tue, 24 Jan 2017, Thomas Huth wrote: > Date: Tue, 24 Jan 2017 21:32:44 +0100 > From: Thomas Huth <thuth@redhat.com> > To: Valentin Plotkin <caliborn@SDF.ORG>, qemu-trivial@nongnu.org > Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org > Subject: Re: [Qemu-ppc] [PATCH] PPC: MMU compatibility check. > > On 24.01.2017 19:56, Valentin Plotkin wrote: >> >> Hi everyone, >> >> I looked at the "qemu-system-ppc -nographic -cpu G2leGP3 -M ppce500" on >> the BiteSizedTasks page. The segfault was caused by machine >> initialization code which expected a certain MMU model, checked, so >> unused SPR were read, returning zeros. bamboo and virtex machines are >> affected as well, but it doesn't always cause segfault, usually running >> into unmapped memory and failing somewhat more nicely. >> >> I added the checks. It would be possible to add support for other MMU >> models, but I'm not sure if there is any point (would any guest OS >> support mutually exclusive CPU and machine)? > > Hi, > > great to have a fix for this crash! I don't think it make much sense to > add support for other MMU models here, so the simple checks should be > good enough. > However, your new code obviously does not follow the QEMU coding style. > Could you please run your patch through scripts/checkpatch.pl and fix > all issues that it reports? And when you resubmit, please make sure to > copy the maintainers on CC: as well (scripts/get_maintainers.pl is your > friend here). > > Thanks, > Thomas > > Here is fengshuised version (at least I hope so). Thanks for guiding me. Signed-off-by: Valentin Plotkin <caliborn@sdf.org> --- hw/ppc/e500.c | 6 ++++++ hw/ppc/ppc440_bamboo.c | 6 ++++++ hw/ppc/virtex_ml507.c | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index cf8b122..683d9a9 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -631,6 +631,12 @@ static uint64_t mmubooke_initial_mapsize(CPUPPCState *env) static void mmubooke_create_initial_mapping(CPUPPCState *env) { + if (env->mmu_model != POWERPC_MMU_BOOKE206) { + fprintf(stderr, "MMU model %i not supported by this machine.\n", + env->mmu_model); + exit(-1); + } + ppcmas_tlb_t *tlb = booke206_get_tlbm(env, 1, 0, 0); hwaddr size; int ps; diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c index 5c535b1..793b758 100644 --- a/hw/ppc/ppc440_bamboo.c +++ b/hw/ppc/ppc440_bamboo.c @@ -124,6 +124,12 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env, target_ulong va, hwaddr pa) { + if (env->mmu_model != POWERPC_MMU_BOOKE) { + fprintf(stderr, "MMU model %i not supported by this machine.\n", + env->mmu_model); + exit(-1); + } + ppcemb_tlb_t *tlb = &env->tlb.tlbe[0]; tlb->attr = 0; diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c index b97d966..c01415c 100644 --- a/hw/ppc/virtex_ml507.c +++ b/hw/ppc/virtex_ml507.c @@ -69,6 +69,12 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env, target_ulong va, hwaddr pa) { + if (env->mmu_model != POWERPC_MMU_BOOKE) { + fprintf(stderr, "MMU model %i not supported by this machine.\n", + env->mmu_model); + exit(-1); + } + ppcemb_tlb_t *tlb = &env->tlb.tlbe[0]; tlb->attr = 0; -- 2.5.5 caliborn@sdf.org SDF Public Access UNIX System - http://sdf.org ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-trivial] [Qemu-ppc] [PATCH] PPC: MMU compatibility check. 2017-01-24 21:41 ` Valentin Plotkin @ 2017-01-25 8:12 ` Thomas Huth 0 siblings, 0 replies; 4+ messages in thread From: Thomas Huth @ 2017-01-25 8:12 UTC (permalink / raw) To: Valentin Plotkin Cc: Alexander Graf, David Gibson, Edgar E. Iglesias, qemu-ppc, qemu-devel, qemu-trivial On 24.01.2017 22:41, Valentin Plotkin wrote: > On Tue, 24 Jan 2017, Thomas Huth wrote: > >> Date: Tue, 24 Jan 2017 21:32:44 +0100 >> From: Thomas Huth <thuth@redhat.com> >> To: Valentin Plotkin <caliborn@SDF.ORG>, qemu-trivial@nongnu.org >> Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org >> Subject: Re: [Qemu-ppc] [PATCH] PPC: MMU compatibility check. >> >> On 24.01.2017 19:56, Valentin Plotkin wrote: >>> >>> Hi everyone, >>> >>> I looked at the "qemu-system-ppc -nographic -cpu G2leGP3 -M ppce500" on >>> the BiteSizedTasks page. The segfault was caused by machine >>> initialization code which expected a certain MMU model, checked, so >>> unused SPR were read, returning zeros. bamboo and virtex machines are >>> affected as well, but it doesn't always cause segfault, usually running >>> into unmapped memory and failing somewhat more nicely. >>> >>> I added the checks. It would be possible to add support for other MMU >>> models, but I'm not sure if there is any point (would any guest OS >>> support mutually exclusive CPU and machine)? >> >> Hi, >> >> great to have a fix for this crash! I don't think it make much sense to >> add support for other MMU models here, so the simple checks should be >> good enough. >> However, your new code obviously does not follow the QEMU coding style. >> Could you please run your patch through scripts/checkpatch.pl and fix >> all issues that it reports? And when you resubmit, please make sure to >> copy the maintainers on CC: as well (scripts/get_maintainers.pl is your >> friend here). > > Here is fengshuised version (at least I hope so). Thanks for guiding me. Thanks, code looks better now! Now to get your patch included, you've got to send it in a separate mail, with only the patch description in it, i.e. not as a reply to another mail with the history of the mail thread. Reason for this is simply that the maintainers can pick up your patch with "git am" without having to edit things manually. Please see also http://qemu-project.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message if you haven't read that page yet. Ah, and two more things that came to my mind: 1) Please use "exit(1)" (or "exit(EXIT_FAILURE)") instead of "exit(-1)", since that's the more common way to do that (since AFAIK the -1 will be cast to 255 at the shell level, which is just ugly). 2) Maybe it would also be better to do the check right after the cpu_ppc_init() in the machine's init function already, instead of doing it in the mmubooke_create_initial_mapping() function? That way we could be sure that wrong CPUs are blocked right from the start. Thanks, Thomas ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-25 8:12 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-01-24 18:56 [Qemu-trivial] [PATCH] PPC: MMU compatibility check Valentin Plotkin 2017-01-24 20:32 ` [Qemu-trivial] [Qemu-ppc] " Thomas Huth 2017-01-24 21:41 ` Valentin Plotkin 2017-01-25 8:12 ` Thomas Huth
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).