Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <79f3ab2e-ffff-0733-00bb-bab79a51b1e7@gmail.com>
Date:   Sat, 3 Jun 2023 08:54:58 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH] KVM: x86/mmu: Add "never" option to allow sticky
 disabling of nx_huge_pages
To:     Sean Christopherson <seanjc@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, Li RongQing <lirongqing@baidu.com>,
        Yong He <zhuangel570@gmail.com>,
        Kai Huang <kai.huang@intel.com>
References: <20230602005859.784190-1-seanjc@google.com>
 <28bd9d11-282f-bb22-66f5-d3d9165d4adf@gmail.com>
 <ZHoGEwFqx3kJrmJe@google.com>
Content-Language: en-US
From:   Robert Hoo <robert.hoo.linux@gmail.com>
In-Reply-To: <ZHoGEwFqx3kJrmJe@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 6/2/2023 11:09 PM, Sean Christopherson wrote:
> On Fri, Jun 02, 2023, Robert Hoo wrote:
>> On 6/2/2023 8:58 AM, Sean Christopherson wrote:
>>> @@ -6860,15 +6871,29 @@ static int set_nx_huge_pages(const char *val, const struct kernel_param *kp)
>>>    	bool old_val = nx_huge_pages;
>>>    	bool new_val;
>>> +	if (nx_hugepage_mitigation_hard_disabled)
>>> +		return -EPERM;
>>> +
>>>    	/* In "auto" mode deploy workaround only if CPU has the bug. */
>>> -	if (sysfs_streq(val, "off"))
>>> +	if (sysfs_streq(val, "off")) {
>>>    		new_val = 0;
>>> -	else if (sysfs_streq(val, "force"))
>>> +	} else if (sysfs_streq(val, "force")) {
>>>    		new_val = 1;
>>> -	else if (sysfs_streq(val, "auto"))
>>> +	} else if (sysfs_streq(val, "auto")) {
>>>    		new_val = get_nx_auto_mode();
>>> -	else if (kstrtobool(val, &new_val) < 0)
>>> +	} else if (sysfs_streq(val, "never")) {
>>> +		new_val = 0;
>>> +
>>> +		mutex_lock(&kvm_lock);
>>> +		if (!list_empty(&vm_list)) {
>>> +			mutex_unlock(&kvm_lock);
>>> +			return -EBUSY;

Ah, right, thanks for explanation.

Reviewed-by: Robert Hoo <robert.hoo.linux@gmail.com>

>>> +		}
>>> +		nx_hugepage_mitigation_hard_disabled = true;
>>> +		mutex_unlock(&kvm_lock);
>>> +	} else if (kstrtobool(val, &new_val) < 0) {
>>>    		return -EINVAL;
>>> +	}
>>>
>>
>> IIUC, (Initially) "auto_off"/"off" --> create some VM --> turn to "never",
>> the created VMs still have those kthreads, but can never be used, until
>> destroyed with VM.
> 
> Shouldn't be able to happen.  The above rejects "never" if vm_list isn't empty,
> i.e. if there are any VMs, and sets nx_hugepage_mitigation_hard_disabled under
> kvm_lock to ensure it can't race with KVM_CREATE_VM.  I forgot to call this out
> in the changelog though