Received-SPF: pass (google.com: domain of linux-kernel+bounces-96391-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <35141245-ce1a-4315-8597-3df4f66168f8@intel.com>
Date: Fri, 8 Mar 2024 13:20:55 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH 1/8] KVM: Document KVM_MAP_MEMORY ioctl
Content-Language: en-US
To: Isaku Yamahata <isaku.yamahata@linux.intel.com>
CC: "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "Yamahata, Isaku"
	<isaku.yamahata@intel.com>, "federico.parola@polito.it"
	<federico.parola@polito.it>, "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"dmatlack@google.com" <dmatlack@google.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "isaku.yamahata@gmail.com"
	<isaku.yamahata@gmail.com>, "michael.roth@amd.com" <michael.roth@amd.com>,
	"seanjc@google.com" <seanjc@google.com>
References: <cover.1709288671.git.isaku.yamahata@intel.com>
 <c50dc98effcba3ff68a033661b2941b777c4fb5c.1709288671.git.isaku.yamahata@intel.com>
 <9f8d8e3b707de3cd879e992a30d646475c608678.camel@intel.com>
 <20240307203340.GI368614@ls.amr.corp.intel.com>
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240307203340.GI368614@ls.amr.corp.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NXFvVE9mSldpNE1oMEx6S3gvaWl0Qzg3RW0xSVFHbUFYVGRRWXkrTTE0M1FJ?=
 =?utf-8?B?K0xtL2prMFRHbFVxUnRubCtSdW83RUY3VW9rMENUMGVHcTcyWXVWME9YNk1y?=
 =?utf-8?B?b0JlVFhqclEzblF0bll4WU1NcEhGODJsbFc5N3R0T2VjU05GZFl4NjVvNmwy?=
 =?utf-8?B?S05zbHdWeVE2WTBLWkpwWGYwYWl2ZXYwanNaa3VQYXl6TVN6eFBvWDk4MUov?=
 =?utf-8?B?RDY2YjVYVkdnbUlNNjczSzg3UFQ2MEpydVVYV3cyZDgrU09pYW5EVVVBTWpC?=
 =?utf-8?B?azFaZlZQSVBSZDBTUGpvQUhlQnBpZ1MvUTRSZW5YVStZcXZyeEtQVlM2SXlo?=
 =?utf-8?B?UUxNTGxkLzZMdmE0aklLdUxWczIzTktENFBBQTFaWmE5ejdwdXc4dWFSMERy?=
 =?utf-8?B?bmNrYVkwVkc1ekJOaEh1aXhTQ3BVRjVGaVBsaVVvL2NEQm5BL1BhdU9iKzNr?=
 =?utf-8?B?T0NYZ1Z4MnJiUU5DemFaWjNiUlY1cEZhTmhQdFpWaklrcFhIY1hsbmxhRkpV?=
 =?utf-8?B?dnZXSXQwL2NlakZFdDRiT2JlbWNKSlo0c1J5cy9hTk5WbkFJQm1sVHYxMDV5?=
 =?utf-8?B?MFZLY3c2OWVvWkFSbytRTmRmMW5NK3BhRWs1Zm12T3dReHQ3dnhIbHRYTzAr?=
 =?utf-8?B?U0VNa1FxQWlPWVhCK1NQcFJwaHhFK2RDaFR2S043NHY5aGtIR2hIM2F4Q214?=
 =?utf-8?B?YWhHMHBBTC9jamF1Mi9NQnhiTGoyMHY2cXc5QkdSWGZ4Y2NVZnRSUGRLU091?=
 =?utf-8?B?MW5wanUzQWhpQWpFSTc1U2x1QzRLOXhGZkRXcHhiME9ab0xhcVU4Zzd1Mzdy?=
 =?utf-8?B?eVRZRnlJc0NNRmtQVW9tYS9DYVZ1LzBNbDIvMXFrb1FXa3RtNWxjam0raWpo?=
 =?utf-8?B?TnFzLzF1UFo3MUNQckhSVkZUZ252elNmMkVFbHFhVUxWd3lERm9IM3ZjczRL?=
 =?utf-8?B?bVUxUkF0dFhpZ1ZhdGZSeEJUckdTdGRzNFpiUlFtVlJKc0hlcnE3a2VibUhk?=
 =?utf-8?B?R2dyZHd3ZWdaOWFaOXdSY3lQNnJTSlBiT1dyOFhhMTRNVFNReHlsK3BvRHRK?=
 =?utf-8?B?Y3k2bGJDY1ppNG11b0JqUi81Y2lmYzVieXgyWkozR1N4MkE3NWxsNnBVVmx3?=
 =?utf-8?B?WGUvWWtoVzgxU1lJRlYzODhsMldjQWhmTGFwQnQ4dFdKZ3EwWkhNOGtEM1ZE?=
 =?utf-8?B?Z2MzYUVhYzM3NDc4TWVqVEg0SmxnWStLYURXTjlvcFhDcGtYYmpkTlhNL3hq?=
 =?utf-8?B?ekpsSlJjN2N6bE1wbWVXdFdjdDBvYUZJOEt3VEZJUy9WQ2Q2bHE1YjhQcnhn?=
 =?utf-8?B?dHdGM2YybElsSndoNUFRb0sycGVVTnRYWG9DVmdUTXk4dVVLM0VwM0xGbmgv?=
 =?utf-8?B?dkpUWWhDazY5QlNweERXZ09KYVY1ZVpPcUVCK1MxK0FEcFhHNThUM1oyUFh2?=
 =?utf-8?B?Qkh1WGgwNUo2OFpsNmlubnFKMi9xUHU2TmllbnV0V1lJd0JEUVMrWUloOG9y?=
 =?utf-8?B?WHBWbys0dUUwVWhvM1NGTlUzcnFrQ2NzdnJuaEVMTDBReU1iQUNpZjYrSmoz?=
 =?utf-8?B?cG03UC9Ydi95Wm9jbWFJa3V5ajB5Y0p3YU8remlyVGZtQW1OQWRQaVlIQ2oz?=
 =?utf-8?B?clNCc1dOWTMvanIxamhYNlFhWHZObjhLT2xkYmJ4S0VOVnJwVnpXQnZuNVR5?=
 =?utf-8?B?TUZhRmplVmgrelEyVmhxbkpYamNMOHprREpsYW5XSGN1SWpuQkpIelAzdzZL?=
 =?utf-8?B?Z1JlUCtDb2h6QytmdTJONVpRd0ZwNW5pYkEvUFJIa3VLRW5KWXoxYWdMR1Ir?=
 =?utf-8?B?bng1RzF2K2xVUnZuVVhkMnYwT05ZQ2dYVm5PVzhyb3VkWXNad3UxS3JjYWs2?=
 =?utf-8?B?dWF5VlRIbVR4WCt6WklMbklBeDhhbVgza1VtZEVDUk5QczBFdDUvRHFGZG0r?=
 =?utf-8?B?bmFvQ1BkN1hsQWlhcUg2djBNbHdGelJGWWJ0ZjQ0QzJYOWZ5OU5yWmNEaVVF?=
 =?utf-8?B?MHh6SVY5a0FGZjRGV3Y0Sm9iOTYvUVpHNWpVRldNcGhNcFVKK1hDc2ZZVGpZ?=
 =?utf-8?B?WCszem5mSC9UcmIrVnROYVI2M3JDTW5oTEh2T040bTVRYXFVSGJ3RTZMRmU0?=
 =?utf-8?Q?c4kAmtQYaLZL7wzj6/86W8Ovm?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a4133e5d-6319-4d29-6249-08dc3f05a46f
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2024 00:21:04.7631
 (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: pTj7iluf7j17R9TaISbPdXLQ1I6Q/gvC/BjNHOAtwg852rDSpEHPrYiB2C0stxTTKsnvFKkuKXoQYMDaHQ5r2w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4794
X-OriginatorOrg: intel.com


>>>   
>>> +4.143 KVM_MAP_MEMORY
>>> +------------------------
>>> +
>>> +:Capability: KVM_CAP_MAP_MEMORY
>>> +:Architectures: none
>>> +:Type: vcpu ioctl
>>
>> I think "vcpu ioctl" means theoretically it can be called on multiple vcpus.
>>
>> What happens in that case?
> 
> Each vcpu can handle the ioctl simaltaneously.  

Not sure whether it is implied, but should we document it can be called 
simultaneously?

Also, I believe this is only supposed to be called before VM starts to 
run?  I think we should document that too.

This is userspace ABI, we need to be explicit on how it is supposed to 
be called from userspace.

Btw, I believe there should be some justification in the changelog why 
this should be a vcpu ioctl().

[...]

>>> +:Parameters: struct kvm_memory_mapping(in/out)
>>> +:Returns: 0 on success, <0 on error
>>> +
>>> +KVM_MAP_MEMORY populates guest memory without running vcpu.
>>> +
>>> +::
>>> +
>>> +  struct kvm_memory_mapping {
>>> +	__u64 base_gfn;
>>> +	__u64 nr_pages;
>>> +	__u64 flags;
>>> +	__u64 source;
>>> +  };
>>> +
>>> +  /* For kvm_memory_mapping:: flags */
>>> +  #define KVM_MEMORY_MAPPING_FLAG_WRITE         _BITULL(0)
>>> +  #define KVM_MEMORY_MAPPING_FLAG_EXEC          _BITULL(1)
>>> +  #define KVM_MEMORY_MAPPING_FLAG_USER          _BITULL(2)
>>
>> I am not sure what's the good of having "FLAG_USER"?
>>
>> This ioctl is called from userspace, thus I think we can just treat this always
>> as user-fault?
> 
> The point is how to emulate kvm page fault as if vcpu caused the kvm page
> fault.  Not we call the ioctl as user context.

Sorry I don't quite follow.  What's wrong if KVM just append the #PF 
USER error bit before it calls into the fault handler?

My question is, since this is ABI, you have to tell how userspace is 
supposed to use this.  Maybe I am missing something, but I don't see how 
USER should be used here.