All of lore.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: peng.hao2@zte.com.cn, richard.weiyang@gmail.com
Cc: penghao122@sina.com.cn, pbonzini@redhat.com, rkrcmar@redhat.com,
	tglx@linutronix.de, mingo@redhat.com, joro@8bytes.org,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	x86@kernel.org
Subject: Re: [PATCH] kvm/x86 : avoid shifting signed 32-bit value by 31 bits
Date: Mon, 15 Oct 2018 10:16:50 -0700	[thread overview]
Message-ID: <717c02f3-bb36-33c4-a463-f5759fde30fd@zytor.com> (raw)
In-Reply-To: <201810080904344038939@zte.com.cn>

On 10/7/18 6:04 PM, peng.hao2@zte.com.cn wrote:
\>>>>>
>>>>> #define AVIC_LOGICAL_ID_ENTRY_GUEST_PHYSICAL_ID_MASK    (0xFF)
>>>>> -#define AVIC_LOGICAL_ID_ENTRY_VALID_MASK        (1 << 31)
>>>>> +#define AVIC_LOGICAL_ID_ENTRY_VALID_MASK        (1UL << 31)
>>>
>>>> It is reasonable to change to unsigned, while not necessary to unsigned
>>>> long?
>>> AVIC_LOGICAL_ID_ENTRY_VALID_MASK is used in function avic_ldr_write.
>>> here I think it doesn't matter if you use unsigned or unsigned long. Do you have any suggestions?
> 
>> In current case, AVIC_LOGICAL_ID_ENTRY_VALID_MASK is used to calculate
>> the value of new_entry with type of u32. So the definition here is not
>> harmful.
> 
>> Also, I did a quick grep and found similar definition (1 << 31) is popular
>> in the whole kernel tree.
> 
>> The reason to make this change is not that strong to me. Would you
>> minding sharing more reason behind this change?
> oh, I'm just thinking logically, not more reason.

The right way to do this would be to use the _BITUL() (or _BITULL()) macro.

	-hpa


  parent reply	other threads:[~2018-10-15 17:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-04 17:47 [PATCH] kvm/x86 : avoid shifting signed 32-bit value by 31 bits Peng Hao
2018-10-06  2:11 ` Wei Yang
     [not found]   ` <201810061131044868733@zte.com.cn>
2018-10-06  7:08     ` Wei Yang
     [not found]       ` <201810080904344038939@zte.com.cn>
2018-10-08  2:25         ` Wei Yang
2018-10-15 17:12           ` Paolo Bonzini
2018-10-15 17:16         ` H. Peter Anvin [this message]
2018-10-15 17:23           ` Paolo Bonzini
2018-10-15 17:30             ` H. Peter Anvin

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=717c02f3-bb36-33c4-a463-f5759fde30fd@zytor.com \
    --to=hpa@zytor.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peng.hao2@zte.com.cn \
    --cc=penghao122@sina.com.cn \
    --cc=richard.weiyang@gmail.com \
    --cc=rkrcmar@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.