From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kai Huang Subject: Re: [v2 05/11] vmx: add new data structure member to support PML Date: Fri, 17 Apr 2015 10:31:26 +0800 Message-ID: <5530707E.4060102@linux.intel.com> References: <1429081433-9600-1-git-send-email-kai.huang@linux.intel.com> <1429081433-9600-6-git-send-email-kai.huang@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Tian, Kevin" , "andrew.cooper3@citrix.com" , "tim@xen.org" , "jbeulich@suse.com" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 04/17/2015 06:39 AM, Tian, Kevin wrote: >> From: Kai Huang [mailto:kai.huang@linux.intel.com] >> Sent: Wednesday, April 15, 2015 3:04 PM >> >> A new 4K page pointer is added to arch_vmx_struct as PML buffer for vcpu. >> And a >> new 'status' field is added to vmx_domain to indicate whether PML is enabled >> for >> the domain or not. The 'status' field also can be used for further similiar >> purpose. > not sure about the last sentence. what's the similar purpose to "whether PML > is enabled"? :-) I mean potentially there might be such feature in the future, and I can't give you an example right now. If you are just commenting the description here but fine with the current code, I can remove that last sentence if you like. Or do you suggest to just use a "bool_t pml_enabled"? I am fine with both, but looks there's no objection from others so I intend to keep it as 'unsigned int status', if you agree. > >> Note both new members don't have to be initialized to zero explicitly as both >> vcpu and domain structure are zero-ed when they are created. > no initialization in this patch, so why explaining it here? OK. Looks it's a common sense to all of you so I'll just remove this sentence. > >> Signed-off-by: Kai Huang >> --- >> xen/include/asm-x86/hvm/vmx/vmcs.h | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h >> b/xen/include/asm-x86/hvm/vmx/vmcs.h >> index f831a78..2c679ac 100644 >> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h >> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h >> @@ -70,8 +70,12 @@ struct ept_data { >> cpumask_var_t synced_mask; >> }; >> >> +#define _VMX_DOMAIN_PML_ENABLED 0 >> +#define VMX_DOMAIN_PML_ENABLED (1ul << >> _VMX_DOMAIN_PML_ENABLED) >> struct vmx_domain { >> unsigned long apic_access_mfn; >> + /* VMX_DOMAIN_* */ >> + unsigned long status; >> }; >> >> struct pi_desc { >> @@ -142,6 +146,9 @@ struct arch_vmx_struct { >> /* Bitmap to control vmexit policy for Non-root VMREAD/VMWRITE */ >> struct page_info *vmread_bitmap; >> struct page_info *vmwrite_bitmap; >> + >> +#define NR_PML_ENTRIES 512 >> + struct page_info *pml_pg; > move the macro out of the structure. OK. I will move it just above the declaration of struct arch_vmx_struct. > and is pml_buffer or pml_buf more clear? To me pml_buffer or pml_buf is more likely a virtual address you can access the buffer directly, while pml_pg indicates it's a pointer of struct page_info. If you you look at patch 6, you can find statements like: uint64_t *pml_buf; pml_buf = __map_domain_page(v->arch.hvm_vmx.pml_pg); So I intend to keep it. Thanks, -Kai > >> }; >> >> int vmx_create_vmcs(struct vcpu *v); >> -- >> 2.1.0 > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel