* [PATCH 0/4] x86: replace plain numbers
@ 2015-01-22 13:53 Jan Beulich
2015-01-22 13:57 ` [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() Jan Beulich
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Jan Beulich @ 2015-01-22 13:53 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser
1: HVM: replace plain number in hvm_combine_hw_exceptions()
2: HVM: replace plain numbers
3: traps: replace plain numbers
4: VMX: replace plain numbers
Signed-off-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() 2015-01-22 13:53 [PATCH 0/4] x86: replace plain numbers Jan Beulich @ 2015-01-22 13:57 ` Jan Beulich 2015-01-22 14:12 ` Andrew Cooper 2015-01-22 14:42 ` Tim Deegan 2015-01-22 13:57 ` [PATCH 2/4] x86/HVM: replace plain numbers Jan Beulich ` (2 subsequent siblings) 3 siblings, 2 replies; 18+ messages in thread From: Jan Beulich @ 2015-01-22 13:57 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Keir Fraser [-- Attachment #1: Type: text/plain, Size: 1407 bytes --] While doing so also take care of #VE here (even if we don't make use of it yet). Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -205,6 +205,16 @@ int hvm_event_needs_reinjection(uint8_t */ uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2) { + const unsigned int contributory_exceptions = + (1 << TRAP_divide_error) | + (1 << TRAP_invalid_tss) | + (1 << TRAP_no_segment) | + (1 << TRAP_stack_error) | + (1 << TRAP_gp_fault); + const unsigned int page_faults = + (1 << TRAP_page_fault) | + (1 << TRAP_virtualisation); + /* Exception during double-fault delivery always causes a triple fault. */ if ( vec1 == TRAP_double_fault ) { @@ -213,11 +223,12 @@ uint8_t hvm_combine_hw_exceptions(uint8_ } /* Exception during page-fault delivery always causes a double fault. */ - if ( vec1 == TRAP_page_fault ) + if ( (1u << vec1) & page_faults ) return TRAP_double_fault; /* Discard the first exception if it's benign or if we now have a #PF. */ - if ( !((1u << vec1) & 0x7c01u) || (vec2 == TRAP_page_fault) ) + if ( !((1u << vec1) & contributory_exceptions) || + ((1u << vec2) & page_faults) ) return vec2; /* Cannot combine the exceptions: double fault. */ [-- Attachment #2: x86-HVM-combine-exceptions.patch --] [-- Type: text/plain, Size: 1465 bytes --] x86/HVM: replace plain number in hvm_combine_hw_exceptions() While doing so also take care of #VE here (even if we don't make use of it yet). Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -205,6 +205,16 @@ int hvm_event_needs_reinjection(uint8_t */ uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2) { + const unsigned int contributory_exceptions = + (1 << TRAP_divide_error) | + (1 << TRAP_invalid_tss) | + (1 << TRAP_no_segment) | + (1 << TRAP_stack_error) | + (1 << TRAP_gp_fault); + const unsigned int page_faults = + (1 << TRAP_page_fault) | + (1 << TRAP_virtualisation); + /* Exception during double-fault delivery always causes a triple fault. */ if ( vec1 == TRAP_double_fault ) { @@ -213,11 +223,12 @@ uint8_t hvm_combine_hw_exceptions(uint8_ } /* Exception during page-fault delivery always causes a double fault. */ - if ( vec1 == TRAP_page_fault ) + if ( (1u << vec1) & page_faults ) return TRAP_double_fault; /* Discard the first exception if it's benign or if we now have a #PF. */ - if ( !((1u << vec1) & 0x7c01u) || (vec2 == TRAP_page_fault) ) + if ( !((1u << vec1) & contributory_exceptions) || + ((1u << vec2) & page_faults) ) return vec2; /* Cannot combine the exceptions: double fault. */ [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() 2015-01-22 13:57 ` [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() Jan Beulich @ 2015-01-22 14:12 ` Andrew Cooper 2015-01-22 14:36 ` Jan Beulich 2015-01-22 14:42 ` Tim Deegan 1 sibling, 1 reply; 18+ messages in thread From: Andrew Cooper @ 2015-01-22 14:12 UTC (permalink / raw) To: Jan Beulich, xen-devel; +Cc: Keir Fraser On 22/01/15 13:57, Jan Beulich wrote: > While doing so also take care of #VE here (even if we don't make use of > it yet). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -205,6 +205,16 @@ int hvm_event_needs_reinjection(uint8_t > */ > uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2) > { > + const unsigned int contributory_exceptions = > + (1 << TRAP_divide_error) | > + (1 << TRAP_invalid_tss) | > + (1 << TRAP_no_segment) | > + (1 << TRAP_stack_error) | > + (1 << TRAP_gp_fault); > + const unsigned int page_faults = > + (1 << TRAP_page_fault) | > + (1 << TRAP_virtualisation); static as an extra hint? I frankly hope that any decent compiler would turn these into instruction immediate data. Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> (FWIW the 0x7c01 constant was the inspiration for my vlapic patch, although I felt that constructing the map at compile time was better than an opaque number.) > + > /* Exception during double-fault delivery always causes a triple fault. */ > if ( vec1 == TRAP_double_fault ) > { > @@ -213,11 +223,12 @@ uint8_t hvm_combine_hw_exceptions(uint8_ > } > > /* Exception during page-fault delivery always causes a double fault. */ > - if ( vec1 == TRAP_page_fault ) > + if ( (1u << vec1) & page_faults ) > return TRAP_double_fault; > > /* Discard the first exception if it's benign or if we now have a #PF. */ > - if ( !((1u << vec1) & 0x7c01u) || (vec2 == TRAP_page_fault) ) > + if ( !((1u << vec1) & contributory_exceptions) || > + ((1u << vec2) & page_faults) ) > return vec2; > > /* Cannot combine the exceptions: double fault. */ > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() 2015-01-22 14:12 ` Andrew Cooper @ 2015-01-22 14:36 ` Jan Beulich 0 siblings, 0 replies; 18+ messages in thread From: Jan Beulich @ 2015-01-22 14:36 UTC (permalink / raw) To: Andrew Cooper; +Cc: xen-devel, Keir Fraser >>> On 22.01.15 at 15:12, <andrew.cooper3@citrix.com> wrote: > On 22/01/15 13:57, Jan Beulich wrote: >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -205,6 +205,16 @@ int hvm_event_needs_reinjection(uint8_t >> */ >> uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2) >> { >> + const unsigned int contributory_exceptions = >> + (1 << TRAP_divide_error) | >> + (1 << TRAP_invalid_tss) | >> + (1 << TRAP_no_segment) | >> + (1 << TRAP_stack_error) | >> + (1 << TRAP_gp_fault); >> + const unsigned int page_faults = >> + (1 << TRAP_page_fault) | >> + (1 << TRAP_virtualisation); > > static as an extra hint? > > I frankly hope that any decent compiler would turn these into > instruction immediate data. I think the static could actually misguide the compiler to in fact allocate storage. I did verify with gcc 4.9.2 that the compiler does translate the above to literal numbers. Jan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() 2015-01-22 13:57 ` [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() Jan Beulich 2015-01-22 14:12 ` Andrew Cooper @ 2015-01-22 14:42 ` Tim Deegan 2015-01-22 15:12 ` Jan Beulich 1 sibling, 1 reply; 18+ messages in thread From: Tim Deegan @ 2015-01-22 14:42 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Andrew Cooper At 13:57 +0000 on 22 Jan (1421931437), Jan Beulich wrote: > While doing so also take care of #VE here (even if we don't make use of > it yet). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -205,6 +205,16 @@ int hvm_event_needs_reinjection(uint8_t > */ > uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2) > { > + const unsigned int contributory_exceptions = > + (1 << TRAP_divide_error) | > + (1 << TRAP_invalid_tss) | > + (1 << TRAP_no_segment) | > + (1 << TRAP_stack_error) | > + (1 << TRAP_gp_fault); == 0x3c01, i.e. TRAP_page_fault has gone missing. AFAICS that's correct, but please mention it in the patch description. Cheers, Tim. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() 2015-01-22 14:42 ` Tim Deegan @ 2015-01-22 15:12 ` Jan Beulich 2015-01-22 15:27 ` Tim Deegan 0 siblings, 1 reply; 18+ messages in thread From: Jan Beulich @ 2015-01-22 15:12 UTC (permalink / raw) To: Tim Deegan; +Cc: Andrew Cooper, Keir Fraser, xen-devel >>> On 22.01.15 at 15:42, <tim@xen.org> wrote: > At 13:57 +0000 on 22 Jan (1421931437), Jan Beulich wrote: >> While doing so also take care of #VE here (even if we don't make use of >> it yet). >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -205,6 +205,16 @@ int hvm_event_needs_reinjection(uint8_t >> */ >> uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2) >> { >> + const unsigned int contributory_exceptions = >> + (1 << TRAP_divide_error) | >> + (1 << TRAP_invalid_tss) | >> + (1 << TRAP_no_segment) | >> + (1 << TRAP_stack_error) | >> + (1 << TRAP_gp_fault); > > == 0x3c01, i.e. TRAP_page_fault has gone missing. AFAICS that's > correct, but please mention it in the patch description. Since it's part of page_faults I don't think it has gone missing, but I'll add an explaining sentence nevertheless. Jan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() 2015-01-22 15:12 ` Jan Beulich @ 2015-01-22 15:27 ` Tim Deegan 0 siblings, 0 replies; 18+ messages in thread From: Tim Deegan @ 2015-01-22 15:27 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, Keir Fraser, xen-devel At 15:12 +0000 on 22 Jan (1421935935), Jan Beulich wrote: > >>> On 22.01.15 at 15:42, <tim@xen.org> wrote: > > At 13:57 +0000 on 22 Jan (1421931437), Jan Beulich wrote: > >> While doing so also take care of #VE here (even if we don't make use of > >> it yet). > >> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > >> > >> --- a/xen/arch/x86/hvm/hvm.c > >> +++ b/xen/arch/x86/hvm/hvm.c > >> @@ -205,6 +205,16 @@ int hvm_event_needs_reinjection(uint8_t > >> */ > >> uint8_t hvm_combine_hw_exceptions(uint8_t vec1, uint8_t vec2) > >> { > >> + const unsigned int contributory_exceptions = > >> + (1 << TRAP_divide_error) | > >> + (1 << TRAP_invalid_tss) | > >> + (1 << TRAP_no_segment) | > >> + (1 << TRAP_stack_error) | > >> + (1 << TRAP_gp_fault); > > > > == 0x3c01, i.e. TRAP_page_fault has gone missing. AFAICS that's > > correct, but please mention it in the patch description. > > Since it's part of page_faults I don't think it has gone missing, but I'll > add an explaining sentence nevertheless. Thanks. With that, Reviewed-by: TIm Deegan <tim@xen.org> Cheers, Tim. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/4] x86/HVM: replace plain numbers 2015-01-22 13:53 [PATCH 0/4] x86: replace plain numbers Jan Beulich 2015-01-22 13:57 ` [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() Jan Beulich @ 2015-01-22 13:57 ` Jan Beulich 2015-01-22 14:41 ` Andrew Cooper 2015-01-22 14:00 ` [PATCH 3/4] x86/traps: " Jan Beulich 2015-01-22 14:01 ` [PATCH 4/4] VMX: " Jan Beulich 3 siblings, 1 reply; 18+ messages in thread From: Jan Beulich @ 2015-01-22 13:57 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Keir Fraser [-- Attachment #1: Type: text/plain, Size: 2973 bytes --] ... making the code better document itself. No functional change intended. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/vioapic.c +++ b/xen/arch/x86/hvm/vioapic.c @@ -53,18 +53,26 @@ static uint32_t vioapic_read_indirect(co switch ( vioapic->ioregsel ) { case VIOAPIC_REG_VERSION: - result = ((((VIOAPIC_NUM_PINS-1) & 0xff) << 16) - | (VIOAPIC_VERSION_ID & 0xff)); + result = ((union IO_APIC_reg_01){ + .bits = { .version = VIOAPIC_VERSION_ID, + .entries = VIOAPIC_NUM_PINS - 1 } + }).raw; break; case VIOAPIC_REG_APIC_ID: + /* + * Using union IO_APIC_reg_02 for the ID register too, as + * union IO_APIC_reg_00's ID field is 8 bits wide for some reason. + */ case VIOAPIC_REG_ARB_ID: - result = ((vioapic->id & 0xf) << 24); + result = ((union IO_APIC_reg_02){ + .bits = { .arbitration = vioapic->id } + }).raw; break; default: { - uint32_t redir_index = (vioapic->ioregsel - 0x10) >> 1; + uint32_t redir_index = (vioapic->ioregsel - VIOAPIC_REG_RTE0) >> 1; uint64_t redir_content; if ( redir_index >= VIOAPIC_NUM_PINS ) @@ -173,7 +181,11 @@ static void vioapic_write_indirect( break; case VIOAPIC_REG_APIC_ID: - vioapic->id = (val >> 24) & 0xf; + /* + * Using union IO_APIC_reg_02 for the ID register, as for some + * reason union IO_APIC_reg_00's ID field is 8 bits wide. + */ + vioapic->id = ((union IO_APIC_reg_02){ .raw = val }).bits.arbitration; break; case VIOAPIC_REG_ARB_ID: @@ -181,7 +193,7 @@ static void vioapic_write_indirect( default: { - uint32_t redir_index = (vioapic->ioregsel - 0x10) >> 1; + uint32_t redir_index = (vioapic->ioregsel - VIOAPIC_REG_RTE0) >> 1; HVM_DBG_LOG(DBG_LEVEL_IOAPIC, "rte[%02x].%s = %08x", redir_index, vioapic->ioregsel & 1 ? "hi" : "lo", val); --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -835,7 +835,7 @@ static int vlapic_write(struct vcpu *v, unsigned int offset = address - vlapic_base_address(vlapic); int rc = X86EMUL_OKAY; - if ( offset != 0xb0 ) + if ( offset != APIC_EOI ) HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "offset %#x with length %#lx, and value is %#lx", offset, len, val); --- a/xen/include/asm-x86/hvm/vioapic.h +++ b/xen/include/asm-x86/hvm/vioapic.h @@ -47,6 +47,7 @@ #define VIOAPIC_REG_APIC_ID 0x00 /* x86 IOAPIC only */ #define VIOAPIC_REG_VERSION 0x01 #define VIOAPIC_REG_ARB_ID 0x02 /* x86 IOAPIC only */ +#define VIOAPIC_REG_RTE0 0x10 struct hvm_vioapic { struct hvm_hw_vioapic hvm_hw_vioapic; [-- Attachment #2: x86-HVM-literals.patch --] [-- Type: text/plain, Size: 3001 bytes --] x86/HVM: replace plain numbers ... making the code better document itself. No functional change intended. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/vioapic.c +++ b/xen/arch/x86/hvm/vioapic.c @@ -53,18 +53,26 @@ static uint32_t vioapic_read_indirect(co switch ( vioapic->ioregsel ) { case VIOAPIC_REG_VERSION: - result = ((((VIOAPIC_NUM_PINS-1) & 0xff) << 16) - | (VIOAPIC_VERSION_ID & 0xff)); + result = ((union IO_APIC_reg_01){ + .bits = { .version = VIOAPIC_VERSION_ID, + .entries = VIOAPIC_NUM_PINS - 1 } + }).raw; break; case VIOAPIC_REG_APIC_ID: + /* + * Using union IO_APIC_reg_02 for the ID register too, as + * union IO_APIC_reg_00's ID field is 8 bits wide for some reason. + */ case VIOAPIC_REG_ARB_ID: - result = ((vioapic->id & 0xf) << 24); + result = ((union IO_APIC_reg_02){ + .bits = { .arbitration = vioapic->id } + }).raw; break; default: { - uint32_t redir_index = (vioapic->ioregsel - 0x10) >> 1; + uint32_t redir_index = (vioapic->ioregsel - VIOAPIC_REG_RTE0) >> 1; uint64_t redir_content; if ( redir_index >= VIOAPIC_NUM_PINS ) @@ -173,7 +181,11 @@ static void vioapic_write_indirect( break; case VIOAPIC_REG_APIC_ID: - vioapic->id = (val >> 24) & 0xf; + /* + * Using union IO_APIC_reg_02 for the ID register, as for some + * reason union IO_APIC_reg_00's ID field is 8 bits wide. + */ + vioapic->id = ((union IO_APIC_reg_02){ .raw = val }).bits.arbitration; break; case VIOAPIC_REG_ARB_ID: @@ -181,7 +193,7 @@ static void vioapic_write_indirect( default: { - uint32_t redir_index = (vioapic->ioregsel - 0x10) >> 1; + uint32_t redir_index = (vioapic->ioregsel - VIOAPIC_REG_RTE0) >> 1; HVM_DBG_LOG(DBG_LEVEL_IOAPIC, "rte[%02x].%s = %08x", redir_index, vioapic->ioregsel & 1 ? "hi" : "lo", val); --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -835,7 +835,7 @@ static int vlapic_write(struct vcpu *v, unsigned int offset = address - vlapic_base_address(vlapic); int rc = X86EMUL_OKAY; - if ( offset != 0xb0 ) + if ( offset != APIC_EOI ) HVM_DBG_LOG(DBG_LEVEL_VLAPIC, "offset %#x with length %#lx, and value is %#lx", offset, len, val); --- a/xen/include/asm-x86/hvm/vioapic.h +++ b/xen/include/asm-x86/hvm/vioapic.h @@ -47,6 +47,7 @@ #define VIOAPIC_REG_APIC_ID 0x00 /* x86 IOAPIC only */ #define VIOAPIC_REG_VERSION 0x01 #define VIOAPIC_REG_ARB_ID 0x02 /* x86 IOAPIC only */ +#define VIOAPIC_REG_RTE0 0x10 struct hvm_vioapic { struct hvm_hw_vioapic hvm_hw_vioapic; [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] x86/HVM: replace plain numbers 2015-01-22 13:57 ` [PATCH 2/4] x86/HVM: replace plain numbers Jan Beulich @ 2015-01-22 14:41 ` Andrew Cooper 2015-01-22 15:17 ` Jan Beulich 0 siblings, 1 reply; 18+ messages in thread From: Andrew Cooper @ 2015-01-22 14:41 UTC (permalink / raw) To: Jan Beulich, xen-devel; +Cc: Keir Fraser On 22/01/15 13:57, Jan Beulich wrote: > ... making the code better document itself. No functional change > intended. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/hvm/vioapic.c > +++ b/xen/arch/x86/hvm/vioapic.c > @@ -53,18 +53,26 @@ static uint32_t vioapic_read_indirect(co > switch ( vioapic->ioregsel ) > { > case VIOAPIC_REG_VERSION: > - result = ((((VIOAPIC_NUM_PINS-1) & 0xff) << 16) > - | (VIOAPIC_VERSION_ID & 0xff)); > + result = ((union IO_APIC_reg_01){ > + .bits = { .version = VIOAPIC_VERSION_ID, > + .entries = VIOAPIC_NUM_PINS - 1 } > + }).raw; > break; > > case VIOAPIC_REG_APIC_ID: > + /* > + * Using union IO_APIC_reg_02 for the ID register too, as > + * union IO_APIC_reg_00's ID field is 8 bits wide for some reason. > + */ Having looked into this, Intel has a 4 bit wide ID with the top 4 bits reserved, while AMD has the top 4 bits as the Extended ID which may be used if an appropriate northbridge register has been set. I think it might be better to use IO_APIC_reg_00 here and mask the write operations, leaving a note about Intel vs AMD and the fact that emulate an Intel IOAPIC to match the PIIX3 chipset in Qemu. Modifying IO_APIC_reg_00 is not appropriate as Xens IOAPIC code needs to deal with AMD systems as well. ~Andrew > case VIOAPIC_REG_ARB_ID: > - result = ((vioapic->id & 0xf) << 24); > + result = ((union IO_APIC_reg_02){ > + .bits = { .arbitration = vioapic->id } > + }).raw; > break; > > default: > { > - uint32_t redir_index = (vioapic->ioregsel - 0x10) >> 1; > + uint32_t redir_index = (vioapic->ioregsel - VIOAPIC_REG_RTE0) >> 1; > uint64_t redir_content; > > if ( redir_index >= VIOAPIC_NUM_PINS ) > @@ -173,7 +181,11 @@ static void vioapic_write_indirect( > break; > > case VIOAPIC_REG_APIC_ID: > - vioapic->id = (val >> 24) & 0xf; > + /* > + * Using union IO_APIC_reg_02 for the ID register, as for some > + * reason union IO_APIC_reg_00's ID field is 8 bits wide. > + */ > + vioapic->id = ((union IO_APIC_reg_02){ .raw = val }).bits.arbitration; > break; > > case VIOAPIC_REG_ARB_ID: > @@ -181,7 +193,7 @@ static void vioapic_write_indirect( > > default: > { > - uint32_t redir_index = (vioapic->ioregsel - 0x10) >> 1; > + uint32_t redir_index = (vioapic->ioregsel - VIOAPIC_REG_RTE0) >> 1; > > HVM_DBG_LOG(DBG_LEVEL_IOAPIC, "rte[%02x].%s = %08x", > redir_index, vioapic->ioregsel & 1 ? "hi" : "lo", val); > --- a/xen/arch/x86/hvm/vlapic.c > +++ b/xen/arch/x86/hvm/vlapic.c > @@ -835,7 +835,7 @@ static int vlapic_write(struct vcpu *v, > unsigned int offset = address - vlapic_base_address(vlapic); > int rc = X86EMUL_OKAY; > > - if ( offset != 0xb0 ) > + if ( offset != APIC_EOI ) > HVM_DBG_LOG(DBG_LEVEL_VLAPIC, > "offset %#x with length %#lx, and value is %#lx", > offset, len, val); > --- a/xen/include/asm-x86/hvm/vioapic.h > +++ b/xen/include/asm-x86/hvm/vioapic.h > @@ -47,6 +47,7 @@ > #define VIOAPIC_REG_APIC_ID 0x00 /* x86 IOAPIC only */ > #define VIOAPIC_REG_VERSION 0x01 > #define VIOAPIC_REG_ARB_ID 0x02 /* x86 IOAPIC only */ > +#define VIOAPIC_REG_RTE0 0x10 > > struct hvm_vioapic { > struct hvm_hw_vioapic hvm_hw_vioapic; > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] x86/HVM: replace plain numbers 2015-01-22 14:41 ` Andrew Cooper @ 2015-01-22 15:17 ` Jan Beulich 2015-01-23 13:41 ` Andrew Cooper 0 siblings, 1 reply; 18+ messages in thread From: Jan Beulich @ 2015-01-22 15:17 UTC (permalink / raw) To: Andrew Cooper; +Cc: xen-devel, Keir Fraser >>> On 22.01.15 at 15:41, <andrew.cooper3@citrix.com> wrote: > On 22/01/15 13:57, Jan Beulich wrote: >> ... making the code better document itself. No functional change >> intended. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/arch/x86/hvm/vioapic.c >> +++ b/xen/arch/x86/hvm/vioapic.c >> @@ -53,18 +53,26 @@ static uint32_t vioapic_read_indirect(co >> switch ( vioapic->ioregsel ) >> { >> case VIOAPIC_REG_VERSION: >> - result = ((((VIOAPIC_NUM_PINS-1) & 0xff) << 16) >> - | (VIOAPIC_VERSION_ID & 0xff)); >> + result = ((union IO_APIC_reg_01){ >> + .bits = { .version = VIOAPIC_VERSION_ID, >> + .entries = VIOAPIC_NUM_PINS - 1 } >> + }).raw; >> break; >> >> case VIOAPIC_REG_APIC_ID: >> + /* >> + * Using union IO_APIC_reg_02 for the ID register too, as >> + * union IO_APIC_reg_00's ID field is 8 bits wide for some reason. >> + */ > > Having looked into this, Intel has a 4 bit wide ID with the top 4 bits > reserved, while AMD has the top 4 bits as the Extended ID which may be > used if an appropriate northbridge register has been set. > > I think it might be better to use IO_APIC_reg_00 here and mask the write > operations, leaving a note about Intel vs AMD and the fact that emulate > an Intel IOAPIC to match the PIIX3 chipset in Qemu. > > Modifying IO_APIC_reg_00 is not appropriate as Xens IOAPIC code needs to > deal with AMD systems as well. I had it that way first, but for the purpose of making very clear that there is no functional change, I decided against doing such a conversion. Jan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] x86/HVM: replace plain numbers 2015-01-22 15:17 ` Jan Beulich @ 2015-01-23 13:41 ` Andrew Cooper 2015-01-23 13:58 ` Jan Beulich 0 siblings, 1 reply; 18+ messages in thread From: Andrew Cooper @ 2015-01-23 13:41 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, Keir Fraser On 22/01/15 15:17, Jan Beulich wrote: >>>> On 22.01.15 at 15:41, <andrew.cooper3@citrix.com> wrote: >> On 22/01/15 13:57, Jan Beulich wrote: >>> ... making the code better document itself. No functional change >>> intended. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> >>> --- a/xen/arch/x86/hvm/vioapic.c >>> +++ b/xen/arch/x86/hvm/vioapic.c >>> @@ -53,18 +53,26 @@ static uint32_t vioapic_read_indirect(co >>> switch ( vioapic->ioregsel ) >>> { >>> case VIOAPIC_REG_VERSION: >>> - result = ((((VIOAPIC_NUM_PINS-1) & 0xff) << 16) >>> - | (VIOAPIC_VERSION_ID & 0xff)); >>> + result = ((union IO_APIC_reg_01){ >>> + .bits = { .version = VIOAPIC_VERSION_ID, >>> + .entries = VIOAPIC_NUM_PINS - 1 } >>> + }).raw; >>> break; >>> >>> case VIOAPIC_REG_APIC_ID: >>> + /* >>> + * Using union IO_APIC_reg_02 for the ID register too, as >>> + * union IO_APIC_reg_00's ID field is 8 bits wide for some reason. >>> + */ >> Having looked into this, Intel has a 4 bit wide ID with the top 4 bits >> reserved, while AMD has the top 4 bits as the Extended ID which may be >> used if an appropriate northbridge register has been set. >> >> I think it might be better to use IO_APIC_reg_00 here and mask the write >> operations, leaving a note about Intel vs AMD and the fact that emulate >> an Intel IOAPIC to match the PIIX3 chipset in Qemu. >> >> Modifying IO_APIC_reg_00 is not appropriate as Xens IOAPIC code needs to >> deal with AMD systems as well. > I had it that way first, but for the purpose of making very clear that > there is no functional change, I decided against doing such a conversion. Ok, but please adjust the comment. "for some reason" is not helpful, whereas "because we emulate an Intel IOAPIC which only has a 4 bit ID field, compared to 8 for AMD" would be better. ~Andrew ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] x86/HVM: replace plain numbers 2015-01-23 13:41 ` Andrew Cooper @ 2015-01-23 13:58 ` Jan Beulich 2015-01-23 13:59 ` Andrew Cooper 0 siblings, 1 reply; 18+ messages in thread From: Jan Beulich @ 2015-01-23 13:58 UTC (permalink / raw) To: Andrew Cooper; +Cc: xen-devel, Keir Fraser >>> On 23.01.15 at 14:41, <andrew.cooper3@citrix.com> wrote: > On 22/01/15 15:17, Jan Beulich wrote: >>>>> On 22.01.15 at 15:41, <andrew.cooper3@citrix.com> wrote: >>> On 22/01/15 13:57, Jan Beulich wrote: >>>> ... making the code better document itself. No functional change >>>> intended. >>>> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>>> >>>> --- a/xen/arch/x86/hvm/vioapic.c >>>> +++ b/xen/arch/x86/hvm/vioapic.c >>>> @@ -53,18 +53,26 @@ static uint32_t vioapic_read_indirect(co >>>> switch ( vioapic->ioregsel ) >>>> { >>>> case VIOAPIC_REG_VERSION: >>>> - result = ((((VIOAPIC_NUM_PINS-1) & 0xff) << 16) >>>> - | (VIOAPIC_VERSION_ID & 0xff)); >>>> + result = ((union IO_APIC_reg_01){ >>>> + .bits = { .version = VIOAPIC_VERSION_ID, >>>> + .entries = VIOAPIC_NUM_PINS - 1 } >>>> + }).raw; >>>> break; >>>> >>>> case VIOAPIC_REG_APIC_ID: >>>> + /* >>>> + * Using union IO_APIC_reg_02 for the ID register too, as >>>> + * union IO_APIC_reg_00's ID field is 8 bits wide for some reason. >>>> + */ >>> Having looked into this, Intel has a 4 bit wide ID with the top 4 bits >>> reserved, while AMD has the top 4 bits as the Extended ID which may be >>> used if an appropriate northbridge register has been set. >>> >>> I think it might be better to use IO_APIC_reg_00 here and mask the write >>> operations, leaving a note about Intel vs AMD and the fact that emulate >>> an Intel IOAPIC to match the PIIX3 chipset in Qemu. >>> >>> Modifying IO_APIC_reg_00 is not appropriate as Xens IOAPIC code needs to >>> deal with AMD systems as well. >> I had it that way first, but for the purpose of making very clear that >> there is no functional change, I decided against doing such a conversion. > > Ok, but please adjust the comment. > > "for some reason" is not helpful, whereas "because we emulate an Intel > IOAPIC which only has a 4 bit ID field, compared to 8 for AMD" would be > better. When I wrote the comment, I didn't have a clue why this was inconsistent. Now we have at least a guess. I'll therefore use your wording with "because" preceded by "presumably": + /* + * Presumably because we emulate an Intel IOAPIC which only has a + * 4 bit ID field (compared to 8 for AMD), using union IO_APIC_reg_02 + * for the ID register (union IO_APIC_reg_00's ID field is 8 bits). + */ Jan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] x86/HVM: replace plain numbers 2015-01-23 13:58 ` Jan Beulich @ 2015-01-23 13:59 ` Andrew Cooper 0 siblings, 0 replies; 18+ messages in thread From: Andrew Cooper @ 2015-01-23 13:59 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, Keir Fraser On 23/01/15 13:58, Jan Beulich wrote: >>>> On 23.01.15 at 14:41, <andrew.cooper3@citrix.com> wrote: >> On 22/01/15 15:17, Jan Beulich wrote: >>>>>> On 22.01.15 at 15:41, <andrew.cooper3@citrix.com> wrote: >>>> On 22/01/15 13:57, Jan Beulich wrote: >>>>> ... making the code better document itself. No functional change >>>>> intended. >>>>> >>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>>>> >>>>> --- a/xen/arch/x86/hvm/vioapic.c >>>>> +++ b/xen/arch/x86/hvm/vioapic.c >>>>> @@ -53,18 +53,26 @@ static uint32_t vioapic_read_indirect(co >>>>> switch ( vioapic->ioregsel ) >>>>> { >>>>> case VIOAPIC_REG_VERSION: >>>>> - result = ((((VIOAPIC_NUM_PINS-1) & 0xff) << 16) >>>>> - | (VIOAPIC_VERSION_ID & 0xff)); >>>>> + result = ((union IO_APIC_reg_01){ >>>>> + .bits = { .version = VIOAPIC_VERSION_ID, >>>>> + .entries = VIOAPIC_NUM_PINS - 1 } >>>>> + }).raw; >>>>> break; >>>>> >>>>> case VIOAPIC_REG_APIC_ID: >>>>> + /* >>>>> + * Using union IO_APIC_reg_02 for the ID register too, as >>>>> + * union IO_APIC_reg_00's ID field is 8 bits wide for some reason. >>>>> + */ >>>> Having looked into this, Intel has a 4 bit wide ID with the top 4 bits >>>> reserved, while AMD has the top 4 bits as the Extended ID which may be >>>> used if an appropriate northbridge register has been set. >>>> >>>> I think it might be better to use IO_APIC_reg_00 here and mask the write >>>> operations, leaving a note about Intel vs AMD and the fact that emulate >>>> an Intel IOAPIC to match the PIIX3 chipset in Qemu. >>>> >>>> Modifying IO_APIC_reg_00 is not appropriate as Xens IOAPIC code needs to >>>> deal with AMD systems as well. >>> I had it that way first, but for the purpose of making very clear that >>> there is no functional change, I decided against doing such a conversion. >> Ok, but please adjust the comment. >> >> "for some reason" is not helpful, whereas "because we emulate an Intel >> IOAPIC which only has a 4 bit ID field, compared to 8 for AMD" would be >> better. > When I wrote the comment, I didn't have a clue why this was > inconsistent. Now we have at least a guess. I'll therefore use > your wording with "because" preceded by "presumably": > > + /* > + * Presumably because we emulate an Intel IOAPIC which only has a > + * 4 bit ID field (compared to 8 for AMD), using union IO_APIC_reg_02 > + * for the ID register (union IO_APIC_reg_00's ID field is 8 bits). > + */ Looks much better. Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/4] x86/traps: replace plain numbers 2015-01-22 13:53 [PATCH 0/4] x86: replace plain numbers Jan Beulich 2015-01-22 13:57 ` [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() Jan Beulich 2015-01-22 13:57 ` [PATCH 2/4] x86/HVM: replace plain numbers Jan Beulich @ 2015-01-22 14:00 ` Jan Beulich 2015-01-22 14:45 ` Andrew Cooper 2015-01-22 14:01 ` [PATCH 4/4] VMX: " Jan Beulich 3 siblings, 1 reply; 18+ messages in thread From: Jan Beulich @ 2015-01-22 14:00 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Keir Fraser [-- Attachment #1: Type: text/plain, Size: 1929 bytes --] ... making the code better document itself. No functional change intended. Note that for now (as we don't support RTM yet) DR_STATUS_RESERVED_ONE and its users don't take DR_NOT_RTM into consideration yet. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -531,9 +531,9 @@ static void instruction_done( regs->eflags &= ~X86_EFLAGS_RF; if ( bpmatch || (regs->eflags & X86_EFLAGS_TF) ) { - current->arch.debugreg[6] |= bpmatch | 0xffff0ff0; + current->arch.debugreg[6] |= bpmatch | DR_STATUS_RESERVED_ONE; if ( regs->eflags & X86_EFLAGS_TF ) - current->arch.debugreg[6] |= 0x4000; + current->arch.debugreg[6] |= DR_STEP; do_guest_trap(TRAP_debug, regs, 0); } } @@ -3872,8 +3872,8 @@ long set_debugreg(struct vcpu *v, unsign * DR6: Bits 4-11,16-31 reserved (set to 1). * Bit 12 reserved (set to 0). */ - value &= 0xffffefff; /* reserved bits => 0 */ - value |= 0xffff0ff0; /* reserved bits => 1 */ + value &= ~DR_STATUS_RESERVED_ZERO; /* reserved bits => 0 */ + value |= DR_STATUS_RESERVED_ONE; /* reserved bits => 1 */ if ( v == curr ) write_debugreg(6, value); break; --- a/xen/include/asm-x86/debugreg.h +++ b/xen/include/asm-x86/debugreg.h @@ -21,6 +21,8 @@ #define DR_STEP (0x4000) /* single-step */ #define DR_SWITCH (0x8000) /* task switch */ #define DR_NOT_RTM (0x10000) /* clear: #BP inside RTM region */ +#define DR_STATUS_RESERVED_ZERO (~0xffffeffful) /* Reserved, read as zero */ +#define DR_STATUS_RESERVED_ONE 0xffff0ff0ul /* Reserved, read as one */ /* Now define a bunch of things for manipulating the control register. The top two bytes of the control register consist of 4 fields of 4 [-- Attachment #2: x86-traps-literals.patch --] [-- Type: text/plain, Size: 1959 bytes --] x86/traps: replace plain numbers ... making the code better document itself. No functional change intended. Note that for now (as we don't support RTM yet) DR_STATUS_RESERVED_ONE and its users don't take DR_NOT_RTM into consideration yet. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -531,9 +531,9 @@ static void instruction_done( regs->eflags &= ~X86_EFLAGS_RF; if ( bpmatch || (regs->eflags & X86_EFLAGS_TF) ) { - current->arch.debugreg[6] |= bpmatch | 0xffff0ff0; + current->arch.debugreg[6] |= bpmatch | DR_STATUS_RESERVED_ONE; if ( regs->eflags & X86_EFLAGS_TF ) - current->arch.debugreg[6] |= 0x4000; + current->arch.debugreg[6] |= DR_STEP; do_guest_trap(TRAP_debug, regs, 0); } } @@ -3872,8 +3872,8 @@ long set_debugreg(struct vcpu *v, unsign * DR6: Bits 4-11,16-31 reserved (set to 1). * Bit 12 reserved (set to 0). */ - value &= 0xffffefff; /* reserved bits => 0 */ - value |= 0xffff0ff0; /* reserved bits => 1 */ + value &= ~DR_STATUS_RESERVED_ZERO; /* reserved bits => 0 */ + value |= DR_STATUS_RESERVED_ONE; /* reserved bits => 1 */ if ( v == curr ) write_debugreg(6, value); break; --- a/xen/include/asm-x86/debugreg.h +++ b/xen/include/asm-x86/debugreg.h @@ -21,6 +21,8 @@ #define DR_STEP (0x4000) /* single-step */ #define DR_SWITCH (0x8000) /* task switch */ #define DR_NOT_RTM (0x10000) /* clear: #BP inside RTM region */ +#define DR_STATUS_RESERVED_ZERO (~0xffffeffful) /* Reserved, read as zero */ +#define DR_STATUS_RESERVED_ONE 0xffff0ff0ul /* Reserved, read as one */ /* Now define a bunch of things for manipulating the control register. The top two bytes of the control register consist of 4 fields of 4 [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] x86/traps: replace plain numbers 2015-01-22 14:00 ` [PATCH 3/4] x86/traps: " Jan Beulich @ 2015-01-22 14:45 ` Andrew Cooper 0 siblings, 0 replies; 18+ messages in thread From: Andrew Cooper @ 2015-01-22 14:45 UTC (permalink / raw) To: Jan Beulich, xen-devel; +Cc: Keir Fraser On 22/01/15 14:00, Jan Beulich wrote: > ... making the code better document itself. No functional change > intended. > > Note that for now (as we don't support RTM yet) DR_STATUS_RESERVED_ONE > and its users don't take DR_NOT_RTM into consideration yet. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -531,9 +531,9 @@ static void instruction_done( > regs->eflags &= ~X86_EFLAGS_RF; > if ( bpmatch || (regs->eflags & X86_EFLAGS_TF) ) > { > - current->arch.debugreg[6] |= bpmatch | 0xffff0ff0; > + current->arch.debugreg[6] |= bpmatch | DR_STATUS_RESERVED_ONE; > if ( regs->eflags & X86_EFLAGS_TF ) > - current->arch.debugreg[6] |= 0x4000; > + current->arch.debugreg[6] |= DR_STEP; > do_guest_trap(TRAP_debug, regs, 0); > } > } > @@ -3872,8 +3872,8 @@ long set_debugreg(struct vcpu *v, unsign > * DR6: Bits 4-11,16-31 reserved (set to 1). > * Bit 12 reserved (set to 0). > */ > - value &= 0xffffefff; /* reserved bits => 0 */ > - value |= 0xffff0ff0; /* reserved bits => 1 */ > + value &= ~DR_STATUS_RESERVED_ZERO; /* reserved bits => 0 */ > + value |= DR_STATUS_RESERVED_ONE; /* reserved bits => 1 */ > if ( v == curr ) > write_debugreg(6, value); > break; > --- a/xen/include/asm-x86/debugreg.h > +++ b/xen/include/asm-x86/debugreg.h > @@ -21,6 +21,8 @@ > #define DR_STEP (0x4000) /* single-step */ > #define DR_SWITCH (0x8000) /* task switch */ > #define DR_NOT_RTM (0x10000) /* clear: #BP inside RTM region */ > +#define DR_STATUS_RESERVED_ZERO (~0xffffeffful) /* Reserved, read as zero */ > +#define DR_STATUS_RESERVED_ONE 0xffff0ff0ul /* Reserved, read as one */ > > /* Now define a bunch of things for manipulating the control register. > The top two bytes of the control register consist of 4 fields of 4 > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/4] VMX: replace plain numbers 2015-01-22 13:53 [PATCH 0/4] x86: replace plain numbers Jan Beulich ` (2 preceding siblings ...) 2015-01-22 14:00 ` [PATCH 3/4] x86/traps: " Jan Beulich @ 2015-01-22 14:01 ` Jan Beulich 2015-01-22 15:21 ` Andrew Cooper 2015-01-23 6:25 ` Tian, Kevin 3 siblings, 2 replies; 18+ messages in thread From: Jan Beulich @ 2015-01-22 14:01 UTC (permalink / raw) To: xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima [-- Attachment #1: Type: text/plain, Size: 8635 bytes --] ... making the code better document itself. No functional change intended. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1406,7 +1406,9 @@ static void __vmx_inject_exception(int t * VM entry]", PRM Vol. 3, 22.6.1 (Interruptibility State). */ - intr_fields = (INTR_INFO_VALID_MASK | (type<<8) | trap); + intr_fields = INTR_INFO_VALID_MASK | + MASK_INSR(type, INTR_INFO_INTR_TYPE_MASK) | + MASK_INSR(trap, INTR_INFO_VECTOR_MASK); if ( error_code != HVM_DELIVER_NO_ERROR_CODE ) { __vmwrite(VM_ENTRY_EXCEPTION_ERROR_CODE, error_code); intr_fields |= INTR_INFO_DELIVER_CODE_MASK; @@ -1430,7 +1432,9 @@ void vmx_inject_extint(int trap, uint8_t PIN_BASED_VM_EXEC_CONTROL); if ( pin_based_cntrl & PIN_BASED_EXT_INTR_MASK ) { nvmx_enqueue_n2_exceptions (v, - INTR_INFO_VALID_MASK | (X86_EVENTTYPE_EXT_INTR<<8) | trap, + INTR_INFO_VALID_MASK | + MASK_INSR(X86_EVENTTYPE_EXT_INTR, INTR_INFO_INTR_TYPE_MASK) | + MASK_INSR(trap, INTR_INFO_VECTOR_MASK), HVM_DELIVER_NO_ERROR_CODE, source); return; } @@ -1449,7 +1453,9 @@ void vmx_inject_nmi(void) PIN_BASED_VM_EXEC_CONTROL); if ( pin_based_cntrl & PIN_BASED_NMI_EXITING ) { nvmx_enqueue_n2_exceptions (v, - INTR_INFO_VALID_MASK | (X86_EVENTTYPE_NMI<<8) | TRAP_nmi, + INTR_INFO_VALID_MASK | + MASK_INSR(X86_EVENTTYPE_NMI, INTR_INFO_INTR_TYPE_MASK) | + MASK_INSR(TRAP_nmi, INTR_INFO_VECTOR_MASK), HVM_DELIVER_NO_ERROR_CODE, hvm_intsrc_nmi); return; } @@ -1487,7 +1493,7 @@ static void vmx_inject_trap(struct hvm_t if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF ) { __restore_debug_registers(curr); - write_debugreg(6, read_debugreg(6) | 0x4000); + write_debugreg(6, read_debugreg(6) | DR_STEP); } if ( cpu_has_monitor_trap_flag ) break; @@ -1502,7 +1508,8 @@ static void vmx_inject_trap(struct hvm_t } if ( unlikely(intr_info & INTR_INFO_VALID_MASK) && - (((intr_info >> 8) & 7) == X86_EVENTTYPE_HW_EXCEPTION) ) + (MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) == + X86_EVENTTYPE_HW_EXCEPTION) ) { _trap.vector = hvm_combine_hw_exceptions( (uint8_t)intr_info, _trap.vector); @@ -1517,7 +1524,9 @@ static void vmx_inject_trap(struct hvm_t nvmx_intercepts_exception(curr, _trap.vector, _trap.error_code) ) { nvmx_enqueue_n2_exceptions (curr, - INTR_INFO_VALID_MASK | (_trap.type<<8) | _trap.vector, + INTR_INFO_VALID_MASK | + MASK_INSR(_trap.type, INTR_INFO_INTR_TYPE_MASK) | + MASK_INSR(_trap.vector, INTR_INFO_VECTOR_MASK), _trap.error_code, hvm_intsrc_none); return; } @@ -1976,8 +1985,11 @@ static int vmx_cr_access(unsigned long e } case VMX_CONTROL_REG_ACCESS_TYPE_LMSW: { unsigned long value = curr->arch.hvm_vcpu.guest_cr[0]; - /* LMSW can: (1) set bits 0-3; (2) clear bits 1-3. */ - value = (value & ~0xe) | ((exit_qualification >> 16) & 0xf); + + /* LMSW can (1) set PE; (2) set or clear MP, EM, and TS. */ + value = (value & ~(X86_CR0_MP|X86_CR0_EM|X86_CR0_TS)) | + (VMX_CONTROL_REG_ACCESS_DATA(exit_qualification) & + (X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS)); HVMTRACE_LONG_1D(LMSW, value); return hvm_set_cr0(value); } @@ -2803,7 +2815,7 @@ void vmx_vmexit_handler(struct cpu_user_ */ __vmread(EXIT_QUALIFICATION, &exit_qualification); HVMTRACE_1D(TRAP_DEBUG, exit_qualification); - write_debugreg(6, exit_qualification | 0xffff0ff0); + write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE); if ( !v->domain->debugger_attached || cpu_has_monitor_trap_flag ) goto exit_and_crash; domain_pause_for_debugger(); @@ -2872,8 +2884,8 @@ void vmx_vmexit_handler(struct cpu_user_ hvm_inject_page_fault(regs->error_code, exit_qualification); break; case TRAP_nmi: - if ( (intr_info & INTR_INFO_INTR_TYPE_MASK) != - (X86_EVENTTYPE_NMI << 8) ) + if ( MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) != + X86_EVENTTYPE_NMI ) goto exit_and_crash; HVMTRACE_0D(NMI); /* Already handled above. */ @@ -2924,7 +2936,8 @@ void vmx_vmexit_handler(struct cpu_user_ * - TSW is a vectored event due to a SW exception or SW interrupt. */ inst_len = ((source != 3) || /* CALL, IRET, or JMP? */ - (idtv_info & (1u<<10))) /* IntrType > 3? */ + (MASK_EXTR(idtv_info, INTR_INFO_INTR_TYPE_MASK) + > 3)) /* IntrType > 3? */ ? get_instruction_length() /* Safe: SDM 3B 23.2.4 */ : 0; if ( (source == 3) && (idtv_info & INTR_INFO_DELIVER_CODE_MASK) ) __vmread(IDT_VECTORING_ERROR_CODE, &ecode); --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -1272,7 +1272,7 @@ static void sync_exception_state(struct if ( !(nvmx->intr.intr_info & INTR_INFO_VALID_MASK) ) return; - switch ( (nvmx->intr.intr_info & INTR_INFO_INTR_TYPE_MASK) >> 8 ) + switch ( MASK_EXTR(nvmx->intr.intr_info, INTR_INFO_INTR_TYPE_MASK) ) { case X86_EVENTTYPE_EXT_INTR: /* rename exit_reason to EXTERNAL_INTERRUPT */ @@ -1327,10 +1327,10 @@ static void nvmx_update_apicv(struct vcp ppr = vlapic_set_ppr(vlapic); WARN_ON((ppr & 0xf0) != (vector & 0xf0)); - status = vector << 8; + status = vector << VMX_GUEST_INTR_STATUS_SVI_OFFSET; rvi = vlapic_has_pending_irq(v); if ( rvi != -1 ) - status |= rvi & 0xff; + status |= rvi & VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK; __vmwrite(GUEST_INTR_STATUS, status); } @@ -2161,7 +2161,8 @@ int nvmx_n2_vmexit_handler(struct cpu_us case EXIT_REASON_EXCEPTION_NMI: { unsigned long intr_info; - u32 valid_mask = (X86_EVENTTYPE_HW_EXCEPTION << 8) | + u32 valid_mask = MASK_INSR(X86_EVENTTYPE_HW_EXCEPTION, + INTR_INFO_INTR_TYPE_MASK) | INTR_INFO_VALID_MASK; u64 exec_bitmap; int vector; @@ -2350,8 +2351,8 @@ int nvmx_n2_vmexit_handler(struct cpu_us u32 mask = 0; __vmread(EXIT_QUALIFICATION, &exit_qualification); - cr = exit_qualification & 0xf; - write = (exit_qualification >> 4) & 3; + cr = VMX_CONTROL_REG_ACCESS_NUM(exit_qualification); + write = VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification); /* also according to guest exec_control */ ctrl = __n2_exec_control(v); @@ -2443,8 +2444,9 @@ int nvmx_n2_vmexit_handler(struct cpu_us u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx, CR0_GUEST_HOST_MASK); __vmread(CR0_READ_SHADOW, &old_val); - old_val &= 0xf; - val = (exit_qualification >> 16) & 0xf; + old_val &= X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS; + val = VMX_CONTROL_REG_ACCESS_DATA(exit_qualification) & + (X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS); changed_bits = old_val ^ val; if ( changed_bits & cr0_gh_mask ) nvcpu->nv_vmexit_pending = 1; --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -207,8 +207,10 @@ static inline unsigned long pi_get_pir(s # define VMX_CONTROL_REG_ACCESS_TYPE_MOV_FROM_CR 1 # define VMX_CONTROL_REG_ACCESS_TYPE_CLTS 2 # define VMX_CONTROL_REG_ACCESS_TYPE_LMSW 3 - /* 10:8 - general purpose register operand */ + /* 11:8 - general purpose register operand */ #define VMX_CONTROL_REG_ACCESS_GPR(eq) (((eq) >> 8) & 0xf) + /* 31:16 - LMSW source data */ +#define VMX_CONTROL_REG_ACCESS_DATA(eq) ((uint32_t)(eq) >> 16) /* * Access Rights [-- Attachment #2: VMX-literals.patch --] [-- Type: text/plain, Size: 8661 bytes --] VMX: replace plain numbers ... making the code better document itself. No functional change intended. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1406,7 +1406,9 @@ static void __vmx_inject_exception(int t * VM entry]", PRM Vol. 3, 22.6.1 (Interruptibility State). */ - intr_fields = (INTR_INFO_VALID_MASK | (type<<8) | trap); + intr_fields = INTR_INFO_VALID_MASK | + MASK_INSR(type, INTR_INFO_INTR_TYPE_MASK) | + MASK_INSR(trap, INTR_INFO_VECTOR_MASK); if ( error_code != HVM_DELIVER_NO_ERROR_CODE ) { __vmwrite(VM_ENTRY_EXCEPTION_ERROR_CODE, error_code); intr_fields |= INTR_INFO_DELIVER_CODE_MASK; @@ -1430,7 +1432,9 @@ void vmx_inject_extint(int trap, uint8_t PIN_BASED_VM_EXEC_CONTROL); if ( pin_based_cntrl & PIN_BASED_EXT_INTR_MASK ) { nvmx_enqueue_n2_exceptions (v, - INTR_INFO_VALID_MASK | (X86_EVENTTYPE_EXT_INTR<<8) | trap, + INTR_INFO_VALID_MASK | + MASK_INSR(X86_EVENTTYPE_EXT_INTR, INTR_INFO_INTR_TYPE_MASK) | + MASK_INSR(trap, INTR_INFO_VECTOR_MASK), HVM_DELIVER_NO_ERROR_CODE, source); return; } @@ -1449,7 +1453,9 @@ void vmx_inject_nmi(void) PIN_BASED_VM_EXEC_CONTROL); if ( pin_based_cntrl & PIN_BASED_NMI_EXITING ) { nvmx_enqueue_n2_exceptions (v, - INTR_INFO_VALID_MASK | (X86_EVENTTYPE_NMI<<8) | TRAP_nmi, + INTR_INFO_VALID_MASK | + MASK_INSR(X86_EVENTTYPE_NMI, INTR_INFO_INTR_TYPE_MASK) | + MASK_INSR(TRAP_nmi, INTR_INFO_VECTOR_MASK), HVM_DELIVER_NO_ERROR_CODE, hvm_intsrc_nmi); return; } @@ -1487,7 +1493,7 @@ static void vmx_inject_trap(struct hvm_t if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF ) { __restore_debug_registers(curr); - write_debugreg(6, read_debugreg(6) | 0x4000); + write_debugreg(6, read_debugreg(6) | DR_STEP); } if ( cpu_has_monitor_trap_flag ) break; @@ -1502,7 +1508,8 @@ static void vmx_inject_trap(struct hvm_t } if ( unlikely(intr_info & INTR_INFO_VALID_MASK) && - (((intr_info >> 8) & 7) == X86_EVENTTYPE_HW_EXCEPTION) ) + (MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) == + X86_EVENTTYPE_HW_EXCEPTION) ) { _trap.vector = hvm_combine_hw_exceptions( (uint8_t)intr_info, _trap.vector); @@ -1517,7 +1524,9 @@ static void vmx_inject_trap(struct hvm_t nvmx_intercepts_exception(curr, _trap.vector, _trap.error_code) ) { nvmx_enqueue_n2_exceptions (curr, - INTR_INFO_VALID_MASK | (_trap.type<<8) | _trap.vector, + INTR_INFO_VALID_MASK | + MASK_INSR(_trap.type, INTR_INFO_INTR_TYPE_MASK) | + MASK_INSR(_trap.vector, INTR_INFO_VECTOR_MASK), _trap.error_code, hvm_intsrc_none); return; } @@ -1976,8 +1985,11 @@ static int vmx_cr_access(unsigned long e } case VMX_CONTROL_REG_ACCESS_TYPE_LMSW: { unsigned long value = curr->arch.hvm_vcpu.guest_cr[0]; - /* LMSW can: (1) set bits 0-3; (2) clear bits 1-3. */ - value = (value & ~0xe) | ((exit_qualification >> 16) & 0xf); + + /* LMSW can (1) set PE; (2) set or clear MP, EM, and TS. */ + value = (value & ~(X86_CR0_MP|X86_CR0_EM|X86_CR0_TS)) | + (VMX_CONTROL_REG_ACCESS_DATA(exit_qualification) & + (X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS)); HVMTRACE_LONG_1D(LMSW, value); return hvm_set_cr0(value); } @@ -2803,7 +2815,7 @@ void vmx_vmexit_handler(struct cpu_user_ */ __vmread(EXIT_QUALIFICATION, &exit_qualification); HVMTRACE_1D(TRAP_DEBUG, exit_qualification); - write_debugreg(6, exit_qualification | 0xffff0ff0); + write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE); if ( !v->domain->debugger_attached || cpu_has_monitor_trap_flag ) goto exit_and_crash; domain_pause_for_debugger(); @@ -2872,8 +2884,8 @@ void vmx_vmexit_handler(struct cpu_user_ hvm_inject_page_fault(regs->error_code, exit_qualification); break; case TRAP_nmi: - if ( (intr_info & INTR_INFO_INTR_TYPE_MASK) != - (X86_EVENTTYPE_NMI << 8) ) + if ( MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) != + X86_EVENTTYPE_NMI ) goto exit_and_crash; HVMTRACE_0D(NMI); /* Already handled above. */ @@ -2924,7 +2936,8 @@ void vmx_vmexit_handler(struct cpu_user_ * - TSW is a vectored event due to a SW exception or SW interrupt. */ inst_len = ((source != 3) || /* CALL, IRET, or JMP? */ - (idtv_info & (1u<<10))) /* IntrType > 3? */ + (MASK_EXTR(idtv_info, INTR_INFO_INTR_TYPE_MASK) + > 3)) /* IntrType > 3? */ ? get_instruction_length() /* Safe: SDM 3B 23.2.4 */ : 0; if ( (source == 3) && (idtv_info & INTR_INFO_DELIVER_CODE_MASK) ) __vmread(IDT_VECTORING_ERROR_CODE, &ecode); --- a/xen/arch/x86/hvm/vmx/vvmx.c +++ b/xen/arch/x86/hvm/vmx/vvmx.c @@ -1272,7 +1272,7 @@ static void sync_exception_state(struct if ( !(nvmx->intr.intr_info & INTR_INFO_VALID_MASK) ) return; - switch ( (nvmx->intr.intr_info & INTR_INFO_INTR_TYPE_MASK) >> 8 ) + switch ( MASK_EXTR(nvmx->intr.intr_info, INTR_INFO_INTR_TYPE_MASK) ) { case X86_EVENTTYPE_EXT_INTR: /* rename exit_reason to EXTERNAL_INTERRUPT */ @@ -1327,10 +1327,10 @@ static void nvmx_update_apicv(struct vcp ppr = vlapic_set_ppr(vlapic); WARN_ON((ppr & 0xf0) != (vector & 0xf0)); - status = vector << 8; + status = vector << VMX_GUEST_INTR_STATUS_SVI_OFFSET; rvi = vlapic_has_pending_irq(v); if ( rvi != -1 ) - status |= rvi & 0xff; + status |= rvi & VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK; __vmwrite(GUEST_INTR_STATUS, status); } @@ -2161,7 +2161,8 @@ int nvmx_n2_vmexit_handler(struct cpu_us case EXIT_REASON_EXCEPTION_NMI: { unsigned long intr_info; - u32 valid_mask = (X86_EVENTTYPE_HW_EXCEPTION << 8) | + u32 valid_mask = MASK_INSR(X86_EVENTTYPE_HW_EXCEPTION, + INTR_INFO_INTR_TYPE_MASK) | INTR_INFO_VALID_MASK; u64 exec_bitmap; int vector; @@ -2350,8 +2351,8 @@ int nvmx_n2_vmexit_handler(struct cpu_us u32 mask = 0; __vmread(EXIT_QUALIFICATION, &exit_qualification); - cr = exit_qualification & 0xf; - write = (exit_qualification >> 4) & 3; + cr = VMX_CONTROL_REG_ACCESS_NUM(exit_qualification); + write = VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification); /* also according to guest exec_control */ ctrl = __n2_exec_control(v); @@ -2443,8 +2444,9 @@ int nvmx_n2_vmexit_handler(struct cpu_us u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx, CR0_GUEST_HOST_MASK); __vmread(CR0_READ_SHADOW, &old_val); - old_val &= 0xf; - val = (exit_qualification >> 16) & 0xf; + old_val &= X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS; + val = VMX_CONTROL_REG_ACCESS_DATA(exit_qualification) & + (X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS); changed_bits = old_val ^ val; if ( changed_bits & cr0_gh_mask ) nvcpu->nv_vmexit_pending = 1; --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -207,8 +207,10 @@ static inline unsigned long pi_get_pir(s # define VMX_CONTROL_REG_ACCESS_TYPE_MOV_FROM_CR 1 # define VMX_CONTROL_REG_ACCESS_TYPE_CLTS 2 # define VMX_CONTROL_REG_ACCESS_TYPE_LMSW 3 - /* 10:8 - general purpose register operand */ + /* 11:8 - general purpose register operand */ #define VMX_CONTROL_REG_ACCESS_GPR(eq) (((eq) >> 8) & 0xf) + /* 31:16 - LMSW source data */ +#define VMX_CONTROL_REG_ACCESS_DATA(eq) ((uint32_t)(eq) >> 16) /* * Access Rights [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] VMX: replace plain numbers 2015-01-22 14:01 ` [PATCH 4/4] VMX: " Jan Beulich @ 2015-01-22 15:21 ` Andrew Cooper 2015-01-23 6:25 ` Tian, Kevin 1 sibling, 0 replies; 18+ messages in thread From: Andrew Cooper @ 2015-01-22 15:21 UTC (permalink / raw) To: Jan Beulich, xen-devel; +Cc: Kevin Tian, Eddie Dong, Jun Nakajima [-- Attachment #1.1: Type: text/plain, Size: 9052 bytes --] On 22/01/15 14:01, Jan Beulich wrote: > ... making the code better document itself. No functional change > intended. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -1406,7 +1406,9 @@ static void __vmx_inject_exception(int t > * VM entry]", PRM Vol. 3, 22.6.1 (Interruptibility State). > */ > > - intr_fields = (INTR_INFO_VALID_MASK | (type<<8) | trap); > + intr_fields = INTR_INFO_VALID_MASK | > + MASK_INSR(type, INTR_INFO_INTR_TYPE_MASK) | > + MASK_INSR(trap, INTR_INFO_VECTOR_MASK); > if ( error_code != HVM_DELIVER_NO_ERROR_CODE ) { > __vmwrite(VM_ENTRY_EXCEPTION_ERROR_CODE, error_code); > intr_fields |= INTR_INFO_DELIVER_CODE_MASK; > @@ -1430,7 +1432,9 @@ void vmx_inject_extint(int trap, uint8_t > PIN_BASED_VM_EXEC_CONTROL); > if ( pin_based_cntrl & PIN_BASED_EXT_INTR_MASK ) { > nvmx_enqueue_n2_exceptions (v, > - INTR_INFO_VALID_MASK | (X86_EVENTTYPE_EXT_INTR<<8) | trap, > + INTR_INFO_VALID_MASK | > + MASK_INSR(X86_EVENTTYPE_EXT_INTR, INTR_INFO_INTR_TYPE_MASK) | > + MASK_INSR(trap, INTR_INFO_VECTOR_MASK), > HVM_DELIVER_NO_ERROR_CODE, source); > return; > } > @@ -1449,7 +1453,9 @@ void vmx_inject_nmi(void) > PIN_BASED_VM_EXEC_CONTROL); > if ( pin_based_cntrl & PIN_BASED_NMI_EXITING ) { > nvmx_enqueue_n2_exceptions (v, > - INTR_INFO_VALID_MASK | (X86_EVENTTYPE_NMI<<8) | TRAP_nmi, > + INTR_INFO_VALID_MASK | > + MASK_INSR(X86_EVENTTYPE_NMI, INTR_INFO_INTR_TYPE_MASK) | > + MASK_INSR(TRAP_nmi, INTR_INFO_VECTOR_MASK), > HVM_DELIVER_NO_ERROR_CODE, hvm_intsrc_nmi); > return; > } > @@ -1487,7 +1493,7 @@ static void vmx_inject_trap(struct hvm_t > if ( guest_cpu_user_regs()->eflags & X86_EFLAGS_TF ) > { > __restore_debug_registers(curr); > - write_debugreg(6, read_debugreg(6) | 0x4000); > + write_debugreg(6, read_debugreg(6) | DR_STEP); > } > if ( cpu_has_monitor_trap_flag ) > break; > @@ -1502,7 +1508,8 @@ static void vmx_inject_trap(struct hvm_t > } > > if ( unlikely(intr_info & INTR_INFO_VALID_MASK) && > - (((intr_info >> 8) & 7) == X86_EVENTTYPE_HW_EXCEPTION) ) > + (MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) == > + X86_EVENTTYPE_HW_EXCEPTION) ) > { > _trap.vector = hvm_combine_hw_exceptions( > (uint8_t)intr_info, _trap.vector); > @@ -1517,7 +1524,9 @@ static void vmx_inject_trap(struct hvm_t > nvmx_intercepts_exception(curr, _trap.vector, _trap.error_code) ) > { > nvmx_enqueue_n2_exceptions (curr, > - INTR_INFO_VALID_MASK | (_trap.type<<8) | _trap.vector, > + INTR_INFO_VALID_MASK | > + MASK_INSR(_trap.type, INTR_INFO_INTR_TYPE_MASK) | > + MASK_INSR(_trap.vector, INTR_INFO_VECTOR_MASK), > _trap.error_code, hvm_intsrc_none); > return; > } > @@ -1976,8 +1985,11 @@ static int vmx_cr_access(unsigned long e > } > case VMX_CONTROL_REG_ACCESS_TYPE_LMSW: { > unsigned long value = curr->arch.hvm_vcpu.guest_cr[0]; > - /* LMSW can: (1) set bits 0-3; (2) clear bits 1-3. */ > - value = (value & ~0xe) | ((exit_qualification >> 16) & 0xf); > + > + /* LMSW can (1) set PE; (2) set or clear MP, EM, and TS. */ > + value = (value & ~(X86_CR0_MP|X86_CR0_EM|X86_CR0_TS)) | > + (VMX_CONTROL_REG_ACCESS_DATA(exit_qualification) & > + (X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS)); > HVMTRACE_LONG_1D(LMSW, value); > return hvm_set_cr0(value); > } > @@ -2803,7 +2815,7 @@ void vmx_vmexit_handler(struct cpu_user_ > */ > __vmread(EXIT_QUALIFICATION, &exit_qualification); > HVMTRACE_1D(TRAP_DEBUG, exit_qualification); > - write_debugreg(6, exit_qualification | 0xffff0ff0); > + write_debugreg(6, exit_qualification | DR_STATUS_RESERVED_ONE); > if ( !v->domain->debugger_attached || cpu_has_monitor_trap_flag ) > goto exit_and_crash; > domain_pause_for_debugger(); > @@ -2872,8 +2884,8 @@ void vmx_vmexit_handler(struct cpu_user_ > hvm_inject_page_fault(regs->error_code, exit_qualification); > break; > case TRAP_nmi: > - if ( (intr_info & INTR_INFO_INTR_TYPE_MASK) != > - (X86_EVENTTYPE_NMI << 8) ) > + if ( MASK_EXTR(intr_info, INTR_INFO_INTR_TYPE_MASK) != > + X86_EVENTTYPE_NMI ) > goto exit_and_crash; > HVMTRACE_0D(NMI); > /* Already handled above. */ > @@ -2924,7 +2936,8 @@ void vmx_vmexit_handler(struct cpu_user_ > * - TSW is a vectored event due to a SW exception or SW interrupt. > */ > inst_len = ((source != 3) || /* CALL, IRET, or JMP? */ > - (idtv_info & (1u<<10))) /* IntrType > 3? */ > + (MASK_EXTR(idtv_info, INTR_INFO_INTR_TYPE_MASK) > + > 3)) /* IntrType > 3? */ > ? get_instruction_length() /* Safe: SDM 3B 23.2.4 */ : 0; > if ( (source == 3) && (idtv_info & INTR_INFO_DELIVER_CODE_MASK) ) > __vmread(IDT_VECTORING_ERROR_CODE, &ecode); > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -1272,7 +1272,7 @@ static void sync_exception_state(struct > if ( !(nvmx->intr.intr_info & INTR_INFO_VALID_MASK) ) > return; > > - switch ( (nvmx->intr.intr_info & INTR_INFO_INTR_TYPE_MASK) >> 8 ) > + switch ( MASK_EXTR(nvmx->intr.intr_info, INTR_INFO_INTR_TYPE_MASK) ) > { > case X86_EVENTTYPE_EXT_INTR: > /* rename exit_reason to EXTERNAL_INTERRUPT */ > @@ -1327,10 +1327,10 @@ static void nvmx_update_apicv(struct vcp > ppr = vlapic_set_ppr(vlapic); > WARN_ON((ppr & 0xf0) != (vector & 0xf0)); > > - status = vector << 8; > + status = vector << VMX_GUEST_INTR_STATUS_SVI_OFFSET; > rvi = vlapic_has_pending_irq(v); > if ( rvi != -1 ) > - status |= rvi & 0xff; > + status |= rvi & VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK; > > __vmwrite(GUEST_INTR_STATUS, status); > } > @@ -2161,7 +2161,8 @@ int nvmx_n2_vmexit_handler(struct cpu_us > case EXIT_REASON_EXCEPTION_NMI: > { > unsigned long intr_info; > - u32 valid_mask = (X86_EVENTTYPE_HW_EXCEPTION << 8) | > + u32 valid_mask = MASK_INSR(X86_EVENTTYPE_HW_EXCEPTION, > + INTR_INFO_INTR_TYPE_MASK) | > INTR_INFO_VALID_MASK; > u64 exec_bitmap; > int vector; > @@ -2350,8 +2351,8 @@ int nvmx_n2_vmexit_handler(struct cpu_us > u32 mask = 0; > > __vmread(EXIT_QUALIFICATION, &exit_qualification); > - cr = exit_qualification & 0xf; > - write = (exit_qualification >> 4) & 3; > + cr = VMX_CONTROL_REG_ACCESS_NUM(exit_qualification); > + write = VMX_CONTROL_REG_ACCESS_TYPE(exit_qualification); > /* also according to guest exec_control */ > ctrl = __n2_exec_control(v); > > @@ -2443,8 +2444,9 @@ int nvmx_n2_vmexit_handler(struct cpu_us > u64 cr0_gh_mask = __get_vvmcs(nvcpu->nv_vvmcx, CR0_GUEST_HOST_MASK); > > __vmread(CR0_READ_SHADOW, &old_val); > - old_val &= 0xf; > - val = (exit_qualification >> 16) & 0xf; > + old_val &= X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS; > + val = VMX_CONTROL_REG_ACCESS_DATA(exit_qualification) & > + (X86_CR0_PE|X86_CR0_MP|X86_CR0_EM|X86_CR0_TS); > changed_bits = old_val ^ val; > if ( changed_bits & cr0_gh_mask ) > nvcpu->nv_vmexit_pending = 1; > --- a/xen/include/asm-x86/hvm/vmx/vmx.h > +++ b/xen/include/asm-x86/hvm/vmx/vmx.h > @@ -207,8 +207,10 @@ static inline unsigned long pi_get_pir(s > # define VMX_CONTROL_REG_ACCESS_TYPE_MOV_FROM_CR 1 > # define VMX_CONTROL_REG_ACCESS_TYPE_CLTS 2 > # define VMX_CONTROL_REG_ACCESS_TYPE_LMSW 3 > - /* 10:8 - general purpose register operand */ > + /* 11:8 - general purpose register operand */ > #define VMX_CONTROL_REG_ACCESS_GPR(eq) (((eq) >> 8) & 0xf) > + /* 31:16 - LMSW source data */ > +#define VMX_CONTROL_REG_ACCESS_DATA(eq) ((uint32_t)(eq) >> 16) > > /* > * Access Rights > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel [-- Attachment #1.2: Type: text/html, Size: 9880 bytes --] [-- Attachment #2: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] VMX: replace plain numbers 2015-01-22 14:01 ` [PATCH 4/4] VMX: " Jan Beulich 2015-01-22 15:21 ` Andrew Cooper @ 2015-01-23 6:25 ` Tian, Kevin 1 sibling, 0 replies; 18+ messages in thread From: Tian, Kevin @ 2015-01-23 6:25 UTC (permalink / raw) To: Jan Beulich, xen-devel; +Cc: Dong, Eddie, Nakajima, Jun > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: Thursday, January 22, 2015 10:01 PM > > ... making the code better document itself. No functional change > intended. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Kevin Tian <kevin.tian@intel.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-01-23 13:59 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-01-22 13:53 [PATCH 0/4] x86: replace plain numbers Jan Beulich 2015-01-22 13:57 ` [PATCH 1/4] x86/HVM: replace plain number in hvm_combine_hw_exceptions() Jan Beulich 2015-01-22 14:12 ` Andrew Cooper 2015-01-22 14:36 ` Jan Beulich 2015-01-22 14:42 ` Tim Deegan 2015-01-22 15:12 ` Jan Beulich 2015-01-22 15:27 ` Tim Deegan 2015-01-22 13:57 ` [PATCH 2/4] x86/HVM: replace plain numbers Jan Beulich 2015-01-22 14:41 ` Andrew Cooper 2015-01-22 15:17 ` Jan Beulich 2015-01-23 13:41 ` Andrew Cooper 2015-01-23 13:58 ` Jan Beulich 2015-01-23 13:59 ` Andrew Cooper 2015-01-22 14:00 ` [PATCH 3/4] x86/traps: " Jan Beulich 2015-01-22 14:45 ` Andrew Cooper 2015-01-22 14:01 ` [PATCH 4/4] VMX: " Jan Beulich 2015-01-22 15:21 ` Andrew Cooper 2015-01-23 6:25 ` Tian, Kevin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.