From: "Andreas Färber" <afaerber@suse.de>
To: li guang <lig.fnst@cn.fujitsu.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
jan.kiszka@siemens.com, green@moxielogic.com,
qemu-devel@nongnu.org, blauwirbel@gmail.com, jcmvbkbc@gmail.com,
edgar.iglesias@gmail.com, gxt@mprc.pku.edu.cn, proljc@gmail.com,
agraf@suse.de, evgenyvoevodin@gmail.com, ehabkost@redhat.com,
sw@weilnetz.de, paul@codesourcery.com, stefanha@redhat.com,
imammedo@redhat.com, rth@twiddle.net, aliguori@us.ibm.com,
Laurent@vivier.eu, michael@walle.cc, qemu-ppc@nongnu.org,
pbonzini@redhat.com, aurelien@aurel32.net
Subject: Re: [Qemu-devel] [PATCH 2/2] target-*/cpu.h: remove cs_base for other targets
Date: Wed, 24 Apr 2013 16:22:01 +0200 [thread overview]
Message-ID: <5177EA89.1080503@suse.de> (raw)
In-Reply-To: <1366789222.20507.20.camel@liguang.fnst.cn.fujitsu.com>
Am 24.04.2013 09:40, schrieb li guang:
> 在 2013-04-24三的 08:36 +0100,Peter Maydell写道:
>> On 24 April 2013 08:32, li guang <lig.fnst@cn.fujitsu.com> wrote:
>>> I think even others want to use something like you said,
>>> it should not 'cs_base', or, it's a bad name.
>>
>> Yes, this is why I said "has a less than helpful name".
>>
>>>>
>>>>>>> --- a/target-sparc/cpu.h
>>>>>>> +++ b/target-sparc/cpu.h
>>>>>>> @@ -715,7 +715,7 @@ trap_state* cpu_tsptr(CPUSPARCState* env);
>>>>>>> #define TB_FLAG_AM_ENABLED (1 << 5)
>>>>>>>
>>>>>>> static inline void cpu_get_tb_cpu_state(CPUSPARCState *env, target_ulong *pc,
>>>>>>> - target_ulong *cs_base, int *flags)
>>>>>>> + int *flags)
>>>>>>> {
>>>>>>> *pc = env->pc;
>>>>>>> *cs_base = env->npc;
>>
>>>> You clearly have a problem with your compile and test
>>>> process then, because it is clear from the patch that
>>>> you've removed the cs_base argument from this function
>>>> but the function still has a use of 'cs_base' in it.
>>>
>>> ???, sorry, where do I miss 'cs_base' removing?
>>
>> Last quoted line of source: "*cs_base = env->npc".
>
> OK, thanks!
> that remove by overshoot script!
Some general reminders:
We're in Soft Freeze, so in general no new big patch series will go into
1.5 unless there's a maintainer willing to take care of it - for i386
there is none, and random code cleanups do not look like something we
must absolutely have in the release last minute. At least no one brought
up on yesterday's call that this is a must-have, so maybe after the
release would be a better time to let people review this?
Whenever you send more than one patch, please include a cover letter.
When you resend a series modified, please include a version such as v2,
v3, etc. and a change log in the cover letter rather than resending with
[updated] or in a way that can't be distinguished at all.
You're expected to assure that your patches compile and don't break
`make check` at least. Repeatedly sending patches that cannot possibly
build okay means you're either overlooking error output from your build
or maybe building the wrong source directory?
Generally, touching the innards of TCG requires a good code
understanding and testing of multiple targets; personally I try to avoid
unnecessary changes for fear of breaking rare corner cases. ;)
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
next prev parent reply other threads:[~2013-04-24 14:22 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-24 1:48 [Qemu-devel] [PATCH 1/2] cpu-exec: rid cs_base of TranslationBlock liguang
2013-04-24 1:48 ` [Qemu-devel] [PATCH 2/2] target-*/cpu.h: remove cs_base for other targets liguang
2013-04-24 7:05 ` Peter Maydell
2013-04-24 7:15 ` li guang
2013-04-24 7:28 ` Peter Maydell
2013-04-24 7:32 ` li guang
2013-04-24 7:36 ` Peter Maydell
2013-04-24 7:40 ` li guang
2013-04-24 14:22 ` Andreas Färber [this message]
2013-04-24 15:40 ` Aurelien Jarno
2013-04-24 15:43 ` Andreas Färber
2013-04-25 0:09 ` li guang
2013-04-24 6:36 ` [Qemu-devel] [PATCH 1/2] cpu-exec: rid cs_base of TranslationBlock Paolo Bonzini
2013-04-24 7:11 ` Aurelien Jarno
2013-04-24 7:25 ` li guang
2013-04-24 7:33 ` Peter Maydell
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=5177EA89.1080503@suse.de \
--to=afaerber@suse.de \
--cc=Laurent@vivier.eu \
--cc=agraf@suse.de \
--cc=aliguori@us.ibm.com \
--cc=aurelien@aurel32.net \
--cc=blauwirbel@gmail.com \
--cc=edgar.iglesias@gmail.com \
--cc=ehabkost@redhat.com \
--cc=evgenyvoevodin@gmail.com \
--cc=green@moxielogic.com \
--cc=gxt@mprc.pku.edu.cn \
--cc=imammedo@redhat.com \
--cc=jan.kiszka@siemens.com \
--cc=jcmvbkbc@gmail.com \
--cc=lig.fnst@cn.fujitsu.com \
--cc=michael@walle.cc \
--cc=paul@codesourcery.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=proljc@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=rth@twiddle.net \
--cc=stefanha@redhat.com \
--cc=sw@weilnetz.de \
/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 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).