Re: [Xen-devel] [v2 05/11] vmx: add new data structure member to support PML
> From: Kai Huang [mailto:kai.hu...@linux.intel.com]
> Sent: Tuesday, April 21, 2015 2:05 PM
>
>
> 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.hu...@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?
yes
>
> >
> >>
> >>> 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_ENABLED0
> >>> +#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'?
>
good to me too.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [v2 05/11] vmx: add new data structure member to support PML
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.hu...@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_ENABLED0
+#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
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [v2 05/11] vmx: add new data structure member to support PML
On 04/17/2015 06:39 AM, Tian, Kevin wrote:
From: Kai Huang [mailto:kai.hu...@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_ENABLED0
+#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
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [v2 05/11] vmx: add new data structure member to support PML
On 04/16/2015 11:33 PM, Jan Beulich wrote:
On 15.04.15 at 09:03, wrote:
--- 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_ENABLED0
+#define VMX_DOMAIN_PML_ENABLED (1ul << _VMX_DOMAIN_PML_ENABLED)
struct vmx_domain {
unsigned long apic_access_mfn;
+/* VMX_DOMAIN_* */
+unsigned long status;
Please us "unsigned int" until 32 bits wouldn't suffice anymore. This
will (on the average) produce slightly smaller code.
Sure.
Thanks,
-Kai
Jan
___
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
Re: [Xen-devel] [v2 05/11] vmx: add new data structure member to support PML
> From: Kai Huang [mailto:kai.hu...@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"? :-)
>
> 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?
>
> 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_ENABLED0
> +#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. and is pml_buffer or pml_buf more clear?
> };
>
> 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
Re: [Xen-devel] [v2 05/11] vmx: add new data structure member to support PML
>>> On 15.04.15 at 09:03, wrote:
> --- 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_ENABLED0
> +#define VMX_DOMAIN_PML_ENABLED (1ul << _VMX_DOMAIN_PML_ENABLED)
> struct vmx_domain {
> unsigned long apic_access_mfn;
> +/* VMX_DOMAIN_* */
> +unsigned long status;
Please us "unsigned int" until 32 bits wouldn't suffice anymore. This
will (on the average) produce slightly smaller code.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [v2 05/11] vmx: add new data structure member to support PML
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.
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.
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_ENABLED0
+#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;
};
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
