Re: [Qemu-devel] [PATCH] qemu-kvm: Add CPUID support for VIA CPU

[BrillyWu]

> > 
> > If I want to submit a patch for the qemu-kvm-git,  should I
> use  "[QEMU-DEVEL][Patch]..." as the subject? Or there are other rules 
> for qemu-kvm upstream? If yes, would you like to tell me?. Thanks!
> 
> If you really have to target qemu-kvm only, then you tagging is fine.
> But this patch does not qualify for such exclusiveness. 
> Rather, your feature should go into upstream's KVM first and will then 
> be merged back into qemu-kvm on next update.

I have submit a patch into upstream's KVM for supporting these features before, 
and the patch has been applied in kvm-git.
Do you mean that I should not submit this patch until the KVM's patch is merged 
back?
 
> > 
> >> On 2011-05-05 05:03, address@hidden wrote:
> >>> When KVM is running on VIA CPU with host cpu's model, the
> >> feautures of
> >>> VIA CPU will be passed into kvm guest by calling the CPUID
> >> instruction
> >>> for Centaur.
> >>>
> >>> Signed-off-by: BrillyWu<address@hidden>
> >>> Signed-off-by: KaryJin<address@hidden>
> >>> ---
> >>>  target-i386/cpu.h   |    7 +++++++
> >>>  target-i386/cpuid.c |   48
> >>> +++++++++++++++++++++++++++++++++++++++++++++++-
> >>
> >> You patch is unfortunately line-wrapped.
> > 
> > Yes, I will be careful the next time.
> > 
> >>> @@ -721,6 +725,9 @@ typedef struct CPUX86State {
> >>>      uint32_t cpuid_ext3_features;
> >>>      uint32_t cpuid_apic_id;
> >>>      int cpuid_vendor_override;
> >>> +    /*Store the results of Centaur's CPUID instructions*/
> >>
> >> Please format comments like this /* comment text */, ie. 
> with blanks
> >> after/before the /* / */.
> > OK, I will check it.
> >>
> >>> +1050,15 @@ void cpu_x86_cpuid(CPUX86State *env, uin
> >>>                     uint32_t *ecx, uint32_t *edx)  {
> >>>      /* test if maximum index reached */
> >>> -    if (index & 0x80000000) {
> >>> +    if ((index & 0xC0000000) == 0xC0000000) {
> >>> + /* Handle the Centaur's CPUID instruction.*
> >>> + * If cpuid_xlevel2 is "0", then put into the*
> >>> + * default case. */
> >>> + if (env->cpuid_xlevel2 == 0)
> >>> +     index = 0xF0000000;
> >>> + else if (index > env->cpuid_xlevel2)
> >>> +     index = env->cpuid_xlevel2;
> >>
> >> Please validate your patch before posting with
> scripts/checkpatch.pl.
> > 
> > OK, I will do it. 
> > I found that space is used to code indent other than tab,
> should I follow it or use tab instead in my patch?
> > If I use space, there are some warnings when using
> scripts/checkpatch.pl to validate the patch. Can I ignore them?
> 
> Generally, the advices checkpatch provides are valid and shall be 
> applied. If you feel like you came across a corner case where the 
> script reports nonsense, please post your findings to qemu-devel.
> 

I use the tab instead of space as code indent, and the checkpatch reports no 
warnings.
Thanks!

> Jan
> 
> --
> Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence 
> Center Embedded Linux
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in the 
> body of a message to address@hidden More majordomo info at  
> http://vger.kernel.org/majordomo-info.html
>