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: Tue, 21 Apr 2015 14:04:54 +0800 Message-ID: <5535E886.7070801@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> <5530707E.4060102@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: <5530707E.4060102@linux.intel.com> 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 10:31 AM, Kai Huang wrote: > > > 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. Hi Kevin, What's your opinion here? Is 'unsigned int status' OK to you? > >> >>> 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. And this one? Are you OK with 'pml_pg'? Thanks, -Kai > > 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 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel