| Message ID | 20250320230339.3897874-1-dmukhin@ford.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] x86/irq: introduce APIC_VECTOR_VALID() | expand |
On 21.03.2025 00:05, dmkhn@proton.me wrote: > Add new macro APIC_VECTOR_VALID() to validate the interrupt vector > range as per [1]. This macro replaces hardcoded checks against the > open-coded value 16 in LAPIC and virtual LAPIC code and simplifies > the code a bit. > > [1] Intel SDM volume 3A > Chapter "ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER" > Section "Valid Interrupt Vectors" > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with ... > --- a/xen/arch/x86/include/asm/apicdef.h > +++ b/xen/arch/x86/include/asm/apicdef.h > @@ -78,6 +78,7 @@ > #define APIC_DM_STARTUP 0x00600 > #define APIC_DM_EXTINT 0x00700 > #define APIC_VECTOR_MASK 0x000FF > +#define APIC_VECTOR_VALID(x) (((x) & APIC_VECTOR_MASK) >= 16) ... line length restrictions respected here. I'll see about taking care of this while committing, provided other x86 maintainers wouldn't prefer this to not go in in the first place (so I'll also give it another day or two). Jan
On 24/03/2025 12:36 pm, Jan Beulich wrote: > On 21.03.2025 00:05, dmkhn@proton.me wrote: >> Add new macro APIC_VECTOR_VALID() to validate the interrupt vector >> range as per [1]. This macro replaces hardcoded checks against the >> open-coded value 16 in LAPIC and virtual LAPIC code and simplifies >> the code a bit. >> >> [1] Intel SDM volume 3A >> Chapter "ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER" >> Section "Valid Interrupt Vectors" >> >> Signed-off-by: Denis Mukhin <dmukhin@ford.com> > Reviewed-by: Jan Beulich <jbeulich@suse.com> > with ... > >> --- a/xen/arch/x86/include/asm/apicdef.h >> +++ b/xen/arch/x86/include/asm/apicdef.h >> @@ -78,6 +78,7 @@ >> #define APIC_DM_STARTUP 0x00600 >> #define APIC_DM_EXTINT 0x00700 >> #define APIC_VECTOR_MASK 0x000FF >> +#define APIC_VECTOR_VALID(x) (((x) & APIC_VECTOR_MASK) >= 16) > ... line length restrictions respected here. I'll see about taking care of > this while committing, provided other x86 maintainers wouldn't prefer this > to not go in in the first place (so I'll also give it another day or two). I'm ok with this change. Unlike v1, it's meaningful in context. ~Andrew
On 20/03/2025 11:05 pm, dmkhn@proton.me wrote: > Add new macro APIC_VECTOR_VALID() to validate the interrupt vector > range as per [1]. This macro replaces hardcoded checks against the > open-coded value 16 in LAPIC and virtual LAPIC code and simplifies > the code a bit. > > [1] Intel SDM volume 3A > Chapter "ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER" > Section "Valid Interrupt Vectors" > > Signed-off-by: Denis Mukhin <dmukhin@ford.com> Would it be possible to adjust your git configuration so committer is set to Denis Mukhin <dmukhin@ford.com> ? Right now, it takes manual fixup to prevent your commits going in as authored by dmkhn@proton.me, and one already slipped through. https://xenbits.xen.org/gitweb/?p=xen.git&a=search&h=refs%2Fheads%2Fstaging&st=author&s=dmkhn%40proton.me Thanks, ~Andrew
On Monday, March 24th, 2025 at 6:51 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > > On 20/03/2025 11:05 pm, dmkhn@proton.me wrote: > > > Add new macro APIC_VECTOR_VALID() to validate the interrupt vector > > range as per [1]. This macro replaces hardcoded checks against the > > open-coded value 16 in LAPIC and virtual LAPIC code and simplifies > > the code a bit. > > > > [1] Intel SDM volume 3A > > Chapter "ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER" > > Section "Valid Interrupt Vectors" > > > > Signed-off-by: Denis Mukhin dmukhin@ford.com > > > Would it be possible to adjust your git configuration so committer is > set to Denis Mukhin dmukhin@ford.com ? > > > Right now, it takes manual fixup to prevent your commits going in as > authored by dmkhn@proton.me, and one already slipped through. > https://xenbits.xen.org/gitweb/?p=xen.git&a=search&h=refs%2Fheads%2Fstaging&st=author&s=dmkhn%40proton.me I will fix my environment, sorry for inconvenience. Thanks! > > Thanks, > > ~Andrew
On Monday, March 24th, 2025 at 5:36 AM, Jan Beulich <jbeulich@suse.com> wrote: > > > On 21.03.2025 00:05, dmkhn@proton.me wrote: > > > Add new macro APIC_VECTOR_VALID() to validate the interrupt vector > > range as per [1]. This macro replaces hardcoded checks against the > > open-coded value 16 in LAPIC and virtual LAPIC code and simplifies > > the code a bit. > > > > [1] Intel SDM volume 3A > > Chapter "ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER" > > Section "Valid Interrupt Vectors" > > > > Signed-off-by: Denis Mukhin dmukhin@ford.com > > > Reviewed-by: Jan Beulich jbeulich@suse.com > > with ... > > > --- a/xen/arch/x86/include/asm/apicdef.h > > +++ b/xen/arch/x86/include/asm/apicdef.h > > @@ -78,6 +78,7 @@ > > #define APIC_DM_STARTUP 0x00600 > > #define APIC_DM_EXTINT 0x00700 > > #define APIC_VECTOR_MASK 0x000FF > > +#define APIC_VECTOR_VALID(x) (((x) & APIC_VECTOR_MASK) >= 16) > > > ... line length restrictions respected here. I'll see about taking care of > this while committing, provided other x86 maintainers wouldn't prefer this > to not go in in the first place (so I'll also give it another day or two). Thanks! > > Jan
On Mon, 24 Mar 2025, Denis Mukhin wrote: > On Monday, March 24th, 2025 at 6:51 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > > > > > > On 20/03/2025 11:05 pm, dmkhn@proton.me wrote: > > > > > Add new macro APIC_VECTOR_VALID() to validate the interrupt vector > > > range as per [1]. This macro replaces hardcoded checks against the > > > open-coded value 16 in LAPIC and virtual LAPIC code and simplifies > > > the code a bit. > > > > > > [1] Intel SDM volume 3A > > > Chapter "ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER" > > > Section "Valid Interrupt Vectors" > > > > > > Signed-off-by: Denis Mukhin dmukhin@ford.com > > > > > > Would it be possible to adjust your git configuration so committer is > > set to Denis Mukhin dmukhin@ford.com ? > > > > > > Right now, it takes manual fixup to prevent your commits going in as > > authored by dmkhn@proton.me, and one already slipped through. > > https://xenbits.xen.org/gitweb/?p=xen.git&a=search&h=refs%2Fheads%2Fstaging&st=author&s=dmkhn%40proton.me > > I will fix my environment, sorry for inconvenience. > Thanks! Hi Denis, FYI it is suffiecient to add: From: Denis Mukhin <dmukhin@ford.com> as part of the body of the email (first line), git will automatically pick it up for the Author field. You don't necessarily need to change your email setup.
diff --git a/xen/arch/x86/cpu/mcheck/mce_intel.c b/xen/arch/x86/cpu/mcheck/mce_intel.c index 07b50f8793..1e52b1ac25 100644 --- a/xen/arch/x86/cpu/mcheck/mce_intel.c +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c @@ -135,8 +135,7 @@ static void intel_init_thermal(struct cpuinfo_x86 *c) * BIOS has programmed on AP based on BSP's info we saved (since BIOS * is required to set the same value for all threads/cores). */ - if ( (val & APIC_DM_MASK) != APIC_DM_FIXED - || (val & APIC_VECTOR_MASK) > 0xf ) + if ( (val & APIC_DM_MASK) != APIC_DM_FIXED || APIC_VECTOR_VALID(val) ) apic_write(APIC_LVTTHMR, val); if ( (msr_content & (1ULL<<3)) diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 065b2aab5b..993e972cd7 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -123,7 +123,7 @@ static void vlapic_error(struct vlapic *vlapic, unsigned int err_bit) * will end up back here. Break the cycle by only injecting LVTERR * if it will succeed, and folding in RECVILL otherwise. */ - if ( (lvterr & APIC_VECTOR_MASK) >= 16 ) + if ( APIC_VECTOR_VALID(lvterr) ) inj = true; else set_bit(ilog2(APIC_ESR_RECVILL), &vlapic->hw.pending_esr); @@ -136,7 +136,7 @@ static void vlapic_error(struct vlapic *vlapic, unsigned int err_bit) bool vlapic_test_irq(const struct vlapic *vlapic, uint8_t vec) { - if ( unlikely(vec < 16) ) + if ( unlikely(!APIC_VECTOR_VALID(vec)) ) return false; if ( hvm_funcs.test_pir && @@ -150,7 +150,7 @@ void vlapic_set_irq(struct vlapic *vlapic, uint8_t vec, uint8_t trig) { struct vcpu *target = vlapic_vcpu(vlapic); - if ( unlikely(vec < 16) ) + if ( unlikely(!APIC_VECTOR_VALID(vec)) ) { vlapic_error(vlapic, ilog2(APIC_ESR_RECVILL)); return; @@ -523,7 +523,7 @@ void vlapic_ipi( struct vlapic *target = vlapic_lowest_prio( vlapic_domain(vlapic), vlapic, short_hand, dest, dest_mode); - if ( unlikely((icr_low & APIC_VECTOR_MASK) < 16) ) + if ( unlikely(!APIC_VECTOR_VALID(icr_low)) ) vlapic_error(vlapic, ilog2(APIC_ESR_SENDILL)); else if ( target ) vlapic_accept_irq(vlapic_vcpu(target), icr_low); @@ -531,7 +531,7 @@ void vlapic_ipi( } case APIC_DM_FIXED: - if ( unlikely((icr_low & APIC_VECTOR_MASK) < 16) ) + if ( unlikely(!APIC_VECTOR_VALID(icr_low)) ) { vlapic_error(vlapic, ilog2(APIC_ESR_SENDILL)); break; diff --git a/xen/arch/x86/include/asm/apicdef.h b/xen/arch/x86/include/asm/apicdef.h index 49e29ec801..7b431190d2 100644 --- a/xen/arch/x86/include/asm/apicdef.h +++ b/xen/arch/x86/include/asm/apicdef.h @@ -78,6 +78,7 @@ #define APIC_DM_STARTUP 0x00600 #define APIC_DM_EXTINT 0x00700 #define APIC_VECTOR_MASK 0x000FF +#define APIC_VECTOR_VALID(x) (((x) & APIC_VECTOR_MASK) >= 16) #define APIC_ICR2 0x310 #define GET_xAPIC_DEST_FIELD(x) (((x)>>24)&0xFF) #define SET_xAPIC_DEST_FIELD(x) ((x)<<24)
Add new macro APIC_VECTOR_VALID() to validate the interrupt vector range as per [1]. This macro replaces hardcoded checks against the open-coded value 16 in LAPIC and virtual LAPIC code and simplifies the code a bit. [1] Intel SDM volume 3A Chapter "ADVANCED PROGRAMMABLE INTERRUPT CONTROLLER" Section "Valid Interrupt Vectors" Signed-off-by: Denis Mukhin <dmukhin@ford.com> --- Changes v1->v2: - update APIC_VECTOR_VALID to a macro w/ parameter - CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1727330658 - Link to v1: https://lore.kernel.org/xen-devel/20250315010033.2917197-4-dmukhin@ford.com/ --- xen/arch/x86/cpu/mcheck/mce_intel.c | 3 +-- xen/arch/x86/hvm/vlapic.c | 10 +++++----- xen/arch/x86/include/asm/apicdef.h | 1 + 3 files changed, 7 insertions(+), 7 deletions(-)