On 07.05.2025 08:12, Penny, Zheng wrote: >> -----Original Message----- >> From: Jan Beulich <jbeul...@suse.com> >> Sent: Thursday, April 17, 2025 11:23 PM >> >> On 14.04.2025 09:40, Penny Zheng wrote: >>> --- a/xen/arch/x86/cpu/amd.c >>> +++ b/xen/arch/x86/cpu/amd.c >>> @@ -570,12 +573,35 @@ static void amd_get_topology(struct cpuinfo_x86 *c) >>> : >>> c->cpu_core_id); } >>> >>> +static uint64_t amd_parse_freq(unsigned char c, uint64_t value) >> >> Considering how it's used, does "value" need to be any wider than unsigned >> int? >> What about the return type? > > Value is the value of 64bit PstateDef MSR, although we are only using the > lower 32bit to calculate frequency > Maybe its better to leave it as uint64_t ?
I won't insist, but changing to unsigned int will, I think, produce better code (hence why I suggested the change). > I'll change the return type to unsigned int, and do the following check anyhow > #define INVAL_FREQ_MHZ (~(unsigned int)0) > if ( freq >= UINT_MAX ) > return INVAL_FREQ_MHZ; > else > return (unsigned int) freq; But please avoid such unnecessary "else" (which I think I did ask for before). Jan
