On 10/7/18 6:04 PM, peng.hao2@xxxxxxxxxx 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
