On 2/22/24 18:22, maobibo wrote:
On 2024/2/22 下午5:45, WANG Xuerui wrote:
Hi,
On 2/17/24 11:03, maobibo wrote:
Hi Xuerui,
Good catch, and thank for your patch.
On 2024/2/16 下午4:58, WANG Xuerui wrote:
[snip]
@@ -324,31 +319,33 @@ static int _kvm_get_cpucfg(int id, u64 *v)
if (cpu_has_lasx)
*v |= CPUCFG2_LASX;
- break;
+ return 0;
+ case 0 ... 1:
+ case 3 ... KVM_MAX_CPUCFG_REGS - 1:
+ /* no restrictions on other CPUCFG IDs' values */
+ *v = U64_MAX;
+ return 0;
how about something like this?
default:
/* no restrictions on other CPUCFG IDs' values */
*v = U64_MAX;
return 0;
I don't think this version correctly expresses the intent. Note that
the CPUCFG ID range check is squashed into the switch as well, so one
switch conveniently expresses the three intended cases at once:
* the special treatment of CPUCFG2,
+ case 0 ... 1:
+ case 3 ... KVM_MAX_CPUCFG_REGS - 1:
+ /* no restrictions on other CPUCFG IDs' values */
+ *v = U64_MAX;
+ return 0;
cpucfg6 checking will be added for PMU support soon. So it will be
case 6:
do something check for cpucfg6
return mask;
case 0 ... 1:
case 3 ... 5:
case 7 ... KVM_MAX_CPUCFG_REGS - 1:
*v = U64_MAX;
return 0;
If you think it is reasonable to add these separate "case" sentences, I
have no objection.
* all-allow rules for other in-range CPUCFG IDs, and
* rejection for out-of-range IDs.
static int kvm_check_cpucfg(int id, u64 val)
{
- u64 mask;
- int ret = 0;
-
- if (id < 0 && id >= KVM_MAX_CPUCFG_REGS)
- return -EINVAL;
you can modify && with ||, like this:
if (id < 0 || id >= KVM_MAX_CPUCFG_REGS)
return -EINVAL;
Yes I know. Personally I don't find the "three cases" that annoying, but
I agree with you that it can be mildly frustrating if one case addition
would split the ranges even more.
Given I'd like to also change the U64_MAX into U32_MAX (CPUCFG data is
specified to be 32-bit wide), I'll send a v4 that keeps the "if" branch
to make more people happy (Huacai privately also expressed preference
for minimizing changes to the overall code shape). Thanks for your
suggestion.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
