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: <9fcd1d25-ec4d-eebe-623f-cfeb48b6656f@intel.com>
Date:   Tue, 13 Sep 2022 10:56:13 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.0
Subject: Re: [RFC PATCH 5/9] kvm_main.c: split __kvm_set_memory_region logic
 in kvm_check_mem and kvm_prepare_batch
Content-Language: en-US
To:     Emanuele Giuseppe Esposito <eesposit@redhat.com>
CC:     Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        David Hildenbrand <david@redhat.com>,
        "Maxim Levitsky" <mlevitsk@redhat.com>, <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>, <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>
References: <20220909104506.738478-1-eesposit@redhat.com>
 <20220909104506.738478-6-eesposit@redhat.com>
From:   "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <20220909104506.738478-6-eesposit@redhat.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MGVuVnFhUmlRbVUra0VOdFZTS2FwbmhrbUROYlNsbExza2ZhenRjdFgrT1NO?=
 =?utf-8?B?QXkyUVBYeXRUMW1vbkRSSkR4bUJsWWhrMWhuQ1d1S3NKeTRhYzh5R012UExl?=
 =?utf-8?B?Y1hLZy8rWXJGaTdFYU5FMkpJY0wwNmlRYW9LVFg2TCtMU25Kd1RLREl6Mlda?=
 =?utf-8?B?a3gvWGhhSk41NW1uelo4enRZdlVxNmR3TFNrdUJqZkt4aFRmTGRWaTFJZXNJ?=
 =?utf-8?B?S20yMGxiVGJWRWJucWtQLzg1RS9oQk1jVmhONU9GS1pvZGRzdEVIN1Z0Qnhw?=
 =?utf-8?B?MjgvTEloRWphQkdEcUF0ZWczNWU3bXdjQlRMKzJtRXl2N2N2R2FsbEoraThL?=
 =?utf-8?B?WjVCaWxLa0cyYkRhNlZ5ZGxOYWV2MEhnMDVXdEFXTUJIaDI2Qkt1RHdvNEJQ?=
 =?utf-8?B?aS9tWUhpUlQ2aUtUbGRQUUhGVjNLQldVSzFNOWgveWIyWXY3MjdXeVB2Q3pw?=
 =?utf-8?B?Z2JBdkVNYncvTzRTV01Nc1BEaG1CSUo0T0ZCUUxEZjluZXYvM0lhZXl2cFlT?=
 =?utf-8?B?M3IxMkZQNnNBakxNbkNmcWZTYVVlb1VQNkhrOFovalF1MWdtNU1GQnh2ejNB?=
 =?utf-8?B?OWh5SXNjS1YySlI1N2lRcWRpQ0k3M2FvZnhrVVFhQUZ3MmZsNzlTWmJGL2ZY?=
 =?utf-8?B?ZlFTd1JGVk1vQ3N2alF0cVhPVTlJM281NlQ1TmxRTTUwRXB3Y1NVeDV5WHla?=
 =?utf-8?B?ZEFMRGYzMVF6ZlZTL2xWYis4N3lpbW53SSs1WTMxelM5Z3p3NG5MdTlub25m?=
 =?utf-8?B?ckFUNjFkODRHbGhtaCtSN2xwK2NQQ3NKS1NZN0JQVjFLeGVlckZwM2RSbTNL?=
 =?utf-8?B?MThUdHF2RzJ5RTIreFpnOStKZ2N1NXZObU8wNTVLZ25wbFplaGZ6MlZ4UE9X?=
 =?utf-8?B?Vi9GVktTdVRRZG1QWVk5ZkZBUlJrcEdJSW5GS2hBZURGUTJGRjJmMGhqUSsw?=
 =?utf-8?B?RkdFYWFZMTFPRjNKSkRZOHp6OFFjMTVoejJsK2RDZ2Z5S3dETGdSak1mTVpX?=
 =?utf-8?B?WFZqd1haSCsybG4wWURGRmVTUElWVHZVcW5PaW5vemxGUFFCYmVhdEtEckti?=
 =?utf-8?B?ejFsSDd6S2VZTjhhVEd2SGt1OFJXbHpMVGNmaHYzOVg4eHpaZE9ZT0NmMDcz?=
 =?utf-8?B?Z3k3MlZHbC9LZ1FTdmMrQzJMMVY3VHVNR1lnS3h3TDAyUHZKU2tnbjlhc2hP?=
 =?utf-8?B?ZG9wWHFDbGNLQlVRcTJKekV0clRaeVU2cmV3bE9laEQ5T0ZCQ25SUU04eDhn?=
 =?utf-8?B?YUhkZmlqWi9NRVVFVm1YcWQvbk9VT3MxdmM1cThpeHBRc0tUN2xNem5FRE9S?=
 =?utf-8?B?K0ZmZC9UU0FoMEVSczBybE4xNUNHYjM3OWZxdmo5UWpiUjBPTjRXYmRXaFV4?=
 =?utf-8?B?Q3lwei9KT2Yrbm9NcUhDVmZqaUs2VFlyZkM1VjFBNjN2SnpveldjUFBpSll4?=
 =?utf-8?B?cTFzTmZzTXFON3pnT3FQdk13YkRnOE4zOTNJQzNLTEdJM21rdHVSMDVxUmFw?=
 =?utf-8?B?V3V2cjdXS0xKNDdhaVl2WFgzL0QxZDhrYThkZ1ArUkxaNVhnTlhpdlp0Y1I2?=
 =?utf-8?B?aXZkcmJla2F6UDU5QlBYZUJwcXZGbkYwbVRiNDVYUXQyN2pyK0ZHYm1CTjJD?=
 =?utf-8?B?VEZObU1wSExjdE9GSTZ6SCtDQlp1QTI3eVF5M1Z5bWtBQmN6U2RuTUNTbUQ0?=
 =?utf-8?B?WHh1aEhIVkdTZmdzNVZNQktZalJHQVAva0Q5cjdPTTRqRk93S21FcGJ1dVdS?=
 =?utf-8?B?d0lKTWI0NHRjNERuMld5RVdmeHRpWXo3cFVLTFJEdzdPQXk4V3EycHpkRnpy?=
 =?utf-8?B?WVJGTHA5My9JWXZVR0hRamhSUE9HbnI5UWs1citYODhGdkxkQU84VEdkam5P?=
 =?utf-8?B?bzRVUktQTjd2dm1LQTBqcjNYK3pZMi82ZkhLa042dXFRdjZnUUg4S3pqM1hl?=
 =?utf-8?B?VWtjbG5ja3A4cVJETzZmd0R4ekZEWUNDVDlZTmVPSVBIeGd3SkV4dE1qNlN1?=
 =?utf-8?B?TDNKaHd4S1ZmZHlIS1Q1czRhQjVDK0ZCV2tzZ1JpM3JjM0N1bjVmZXZBeVhC?=
 =?utf-8?B?aXhKR3hTRkQ0b0pJWWhoRzZBYkk3NURRbW84THdCNmRKNSt0c0dldVBub050?=
 =?utf-8?B?NEZHMFdzTHBnNldneWt4M0s5YURSSkdkYkNlY2pRdEFuVjZ4cDJyL3hZcGVn?=
 =?utf-8?B?a2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4423e2e8-f06d-4dc4-614b-08da9533b4a0
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Sep 2022 02:57:33.3719
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: t1t8+u3mxn9HmtFS8fhWmkkqqjjOHeC8IXkI/MwDPdyvvKbPrcIeWJ5MepWo27WXLWmz9DksKjMd0ipk3v9HiQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB5986
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 9/9/2022 6:45 PM, Emanuele Giuseppe Esposito wrote:
> Just a function split. No functional change intended,
> except for the fact that kvm_prepare_batch() does not
> immediately call kvm_set_memslot() if batch->change is
> KVM_MR_DELETE, but delegates the caller (__kvm_set_memory_region).
>
>
> +static int kvm_check_memory_region(struct kvm *kvm,
> +				const struct kvm_userspace_memory_region *mem,
> +				struct kvm_internal_memory_region_list *batch)
> +{
> +	int r;
> +
> +	r = check_memory_region_flags(mem);
> +	if (r)
> +		return r;
>   
> -	r = kvm_set_memslot(kvm, batch);
> +	r = kvm_check_mem(mem);
>   	if (r)
> -		kfree(new);
> +		return r;
> +
> +	r = kvm_prepare_batch(kvm, mem, batch);
> +	if (r && batch->new)
> +		kfree(batch->new);
 From the patch, r !=0 and batch->new !=NULL are exclusive, so free() 
here is not reachable.
> +
>   	return r;
>   }
[...]
>