diff mbox

[PART1,RFC,v2,03/10] svm: Introduce new AVIC VMCB registers

Message ID 1457124368-2025-4-git-send-email-Suravee.Suthikulpanit@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Suthikulpanit, Suravee March 4, 2016, 8:46 p.m. UTC
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>

Introduce new AVIC VMCB registers. Also breakdown int_ctl register
into bit-field for ease of use.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
 arch/x86/include/asm/svm.h | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

Comments

Paolo Bonzini March 7, 2016, 3:44 p.m. UTC | #1
On 04/03/2016 21:46, Suravee Suthikulpanit wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> 
> Introduce new AVIC VMCB registers. Also breakdown int_ctl register
> into bit-field for ease of use.
> 
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
>  arch/x86/include/asm/svm.h | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 6136d99..db5d7af 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -67,10 +67,24 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>  	u32 asid;
>  	u8 tlb_ctl;
>  	u8 reserved_2[3];
> -	u32 int_ctl;
> +	union { /* Offset 0x60 */
> +		u32 int_ctl;
> +
> +		struct __attribute__ ((__packed__)) {
> +			u32 v_tpr		: 8,
> +			    v_irq		: 1,
> +			    reserved_3		: 7,
> +			    v_intr_prio		: 4,
> +			    v_ign_tpr		: 1,
> +			    reserved_4		: 3,
> +			    v_intr_masking	: 1,
> +			    reserved_5		: 6,
> +			    avic_enable		: 1;

Please do not introduce bitfields and drop patch 4.

Thanks,

Paolo

> +		};
> +	};
>  	u32 int_vector;
>  	u32 int_state;
> -	u8 reserved_3[4];
> +	u8 reserved_6[4];
>  	u32 exit_code;
>  	u32 exit_code_hi;
>  	u64 exit_info_1;
> @@ -78,17 +92,22 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>  	u32 exit_int_info;
>  	u32 exit_int_info_err;
>  	u64 nested_ctl;
> -	u8 reserved_4[16];
> +	u64 avic_vapic_bar;
> +	u8 reserved_7[8];
>  	u32 event_inj;
>  	u32 event_inj_err;
>  	u64 nested_cr3;
>  	u64 lbr_ctl;
>  	u32 clean;
> -	u32 reserved_5;
> +	u32 reserved_8;
>  	u64 next_rip;
>  	u8 insn_len;
>  	u8 insn_bytes[15];
> -	u8 reserved_6[800];
> +	u64 avic_bk_page;	/* Offset 0xe0 */
> +	u8 reserved_9[8];	/* Offset 0xe8 */
> +	u64 avic_log_apic_id;	/* Offset 0xf0 */
> +	u64 avic_phy_apic_id;	/* Offset 0xf8 */
> +	u8 reserved_10[768];
>  };
>  
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suthikulpanit, Suravee March 14, 2016, 7:41 a.m. UTC | #2
Hi,

On 03/07/2016 10:44 PM, Paolo Bonzini wrote:
>
> On 04/03/2016 21:46, Suravee Suthikulpanit wrote:
>> >From: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
>> >
>> >Introduce new AVIC VMCB registers. Also breakdown int_ctl register
>> >into bit-field for ease of use.
>> >
>> >Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
>> >---
>> >  arch/x86/include/asm/svm.h | 29 ++++++++++++++++++++++++-----
>> >  1 file changed, 24 insertions(+), 5 deletions(-)
>> >
>> >diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
>> >index 6136d99..db5d7af 100644
>> >--- a/arch/x86/include/asm/svm.h
>> >+++ b/arch/x86/include/asm/svm.h
>> >@@ -67,10 +67,24 @@ struct __attribute__ ((__packed__)) vmcb_control_area {
>> >  	u32 asid;
>> >  	u8 tlb_ctl;
>> >  	u8 reserved_2[3];
>> >-	u32 int_ctl;
>> >+	union { /* Offset 0x60 */
>> >+		u32 int_ctl;
>> >+
>> >+		struct __attribute__ ((__packed__)) {
>> >+			u32 v_tpr		: 8,
>> >+			    v_irq		: 1,
>> >+			    reserved_3		: 7,
>> >+			    v_intr_prio		: 4,
>> >+			    v_ign_tpr		: 1,
>> >+			    reserved_4		: 3,
>> >+			    v_intr_masking	: 1,
>> >+			    reserved_5		: 6,
>> >+			    avic_enable		: 1;
> Please do not introduce bitfields and drop patch 4.
>
> Thanks,
>
> Paolo
>

Any particular reason why you do not recommend the use of bit field?

Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paolo Bonzini March 14, 2016, 12:25 p.m. UTC | #3
On 14/03/2016 08:41, Suravee Suthikulpanit wrote:
> Any particular reason why you do not recommend the use of bit field?

1) The current coding style is generally not using bitfields

2) Having to review patches that change working code unrelated to AVIC

3) Most of the fields are not even used when AVIC is enabled, so the
benefit of the conversion is small.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suthikulpanit, Suravee March 15, 2016, 12:51 p.m. UTC | #4
On 03/14/2016 07:25 PM, Paolo Bonzini wrote:
>
>
> On 14/03/2016 08:41, Suravee Suthikulpanit wrote:
>> Any particular reason why you do not recommend the use of bit field?
>
> 1) The current coding style is generally not using bitfields
>
> 2) Having to review patches that change working code unrelated to AVIC
>
> 3) Most of the fields are not even used when AVIC is enabled, so the
> benefit of the conversion is small.
>
> Paolo
>

Ok I'll remove the bit-field stuff from patch 3 and will get rid off 
patch 4.

Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index 6136d99..db5d7af 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -67,10 +67,24 @@  struct __attribute__ ((__packed__)) vmcb_control_area {
 	u32 asid;
 	u8 tlb_ctl;
 	u8 reserved_2[3];
-	u32 int_ctl;
+	union { /* Offset 0x60 */
+		u32 int_ctl;
+
+		struct __attribute__ ((__packed__)) {
+			u32 v_tpr		: 8,
+			    v_irq		: 1,
+			    reserved_3		: 7,
+			    v_intr_prio		: 4,
+			    v_ign_tpr		: 1,
+			    reserved_4		: 3,
+			    v_intr_masking	: 1,
+			    reserved_5		: 6,
+			    avic_enable		: 1;
+		};
+	};
 	u32 int_vector;
 	u32 int_state;
-	u8 reserved_3[4];
+	u8 reserved_6[4];
 	u32 exit_code;
 	u32 exit_code_hi;
 	u64 exit_info_1;
@@ -78,17 +92,22 @@  struct __attribute__ ((__packed__)) vmcb_control_area {
 	u32 exit_int_info;
 	u32 exit_int_info_err;
 	u64 nested_ctl;
-	u8 reserved_4[16];
+	u64 avic_vapic_bar;
+	u8 reserved_7[8];
 	u32 event_inj;
 	u32 event_inj_err;
 	u64 nested_cr3;
 	u64 lbr_ctl;
 	u32 clean;
-	u32 reserved_5;
+	u32 reserved_8;
 	u64 next_rip;
 	u8 insn_len;
 	u8 insn_bytes[15];
-	u8 reserved_6[800];
+	u64 avic_bk_page;	/* Offset 0xe0 */
+	u8 reserved_9[8];	/* Offset 0xe8 */
+	u64 avic_log_apic_id;	/* Offset 0xf0 */
+	u64 avic_phy_apic_id;	/* Offset 0xf8 */
+	u8 reserved_10[768];
 };