| Message ID | 20250417091708.215826-7-Neeraj.Upadhyay@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | AMD: Add Secure AVIC Guest Support | expand |
On Thu, Apr 17 2025 at 14:46, Neeraj Upadhyay wrote: > Add update_vector callback to set/clear ALLOWED_IRR field in > a vCPU's APIC backing page for external vectors. The ALLOWED_IRR > field indicates the interrupt vectors which the guest allows the > hypervisor to send (typically for emulated devices). Interrupt > vectors used exclusively by the guest itself and the vectors which > are not emulated by the hypervisor, such as IPI vectors, are part > of system vectors and are not set in the ALLOWED_IRR. Please structure changelogs properly in paragraphs: https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog > arch/x86/include/asm/apic.h | 9 +++++ > arch/x86/kernel/apic/vector.c | 53 ++++++++++++++++++++++------- > arch/x86/kernel/apic/x2apic_savic.c | 35 +++++++++++++++++++ And split this patch up into two: 1) Do the modifications in vector.c which is what the $Subject line says 2) Add the SAVIC specific bits > @@ -471,6 +473,12 @@ static __always_inline bool apic_id_valid(u32 apic_id) > return apic_id <= apic->max_apic_id; > } > > +static __always_inline void apic_update_vector(unsigned int cpu, unsigned int vector, bool set) > +{ > + if (apic->update_vector) > + apic->update_vector(cpu, vector, set); > +} This is in the public header because it can? > -static void apic_update_vector(struct irq_data *irqd, unsigned int newvec, > - unsigned int newcpu) > +static int irq_alloc_vector(const struct cpumask *dest, bool resvd, unsigned int *cpu) > +{ > + int vector; > + > + vector = irq_matrix_alloc(vector_matrix, dest, resvd, cpu); int vector = irq_matrix_alloc(...); > + > + if (vector >= 0) > + apic_update_vector(*cpu, vector, true); > + > + return vector; > +} > + > +static int irq_alloc_managed_vector(unsigned int *cpu) > +{ > + int vector; > + > + vector = irq_matrix_alloc_managed(vector_matrix, vector_searchmask, cpu); > + > + if (vector >= 0) > + apic_update_vector(*cpu, vector, true); > + > + return vector; > +} I completely fail to see the value of these two functions. Each of them has exactly _ONE_ call site and both sites invoke apic_update_vector() when the allocation succeeded. Why can't you just do the obvious and leave the existing code alone and add if (apic->update_vector) apic->update_vector(); into apic_update_vector()? But then you have another place where you need the update, which does not invoke apic_update_vector(). Now if you look deeper, then you notice that all places which invoke apic_update_vector() invoke apic_update_irq_cfg(), which is also called at this other place, no? > +static void irq_free_vector(unsigned int cpu, unsigned int vector, bool managed) > +{ > + apic_update_vector(cpu, vector, false); > + irq_matrix_free(vector_matrix, cpu, vector, managed); > +} This one makes sense, but please name it: apic_free_vector() Something like the uncompiled below, no? Thanks, tglx --- --- a/arch/x86/kernel/apic/vector.c +++ b/arch/x86/kernel/apic/vector.c @@ -134,9 +134,19 @@ static void apic_update_irq_cfg(struct i apicd->hw_irq_cfg.vector = vector; apicd->hw_irq_cfg.dest_apicid = apic->calc_dest_apicid(cpu); + + if (apic->update_vector) + apic->update_vector(cpu, vector, true); + irq_data_update_effective_affinity(irqd, cpumask_of(cpu)); - trace_vector_config(irqd->irq, vector, cpu, - apicd->hw_irq_cfg.dest_apicid); + trace_vector_config(irqd->irq, vector, cpu, apicd->hw_irq_cfg.dest_apicid); +} + +static void apic_free_vector(unsigned int cpu, unsigned int vector, bool managed) +{ + if (apic->update_vector) + apic->update_vector(cpu, vector, false); + irq_matrix_free(vector_matrix, cpu, vector, managed); } static void apic_update_vector(struct irq_data *irqd, unsigned int newvec, @@ -174,8 +184,7 @@ static void apic_update_vector(struct ir apicd->prev_cpu = apicd->cpu; WARN_ON_ONCE(apicd->cpu == newcpu); } else { - irq_matrix_free(vector_matrix, apicd->cpu, apicd->vector, - managed); + apic_free_vector(apicd->cpu, apicd->vector, managed); } setnew: @@ -183,6 +192,7 @@ static void apic_update_vector(struct ir apicd->cpu = newcpu; BUG_ON(!IS_ERR_OR_NULL(per_cpu(vector_irq, newcpu)[newvec])); per_cpu(vector_irq, newcpu)[newvec] = desc; + apic_update_irq_cfg(irqd, newvec, newcpu); } static void vector_assign_managed_shutdown(struct irq_data *irqd) @@ -261,8 +271,6 @@ assign_vector_locked(struct irq_data *ir if (vector < 0) return vector; apic_update_vector(irqd, vector, cpu); - apic_update_irq_cfg(irqd, vector, cpu); - return 0; } @@ -338,7 +346,6 @@ assign_managed_vector(struct irq_data *i if (vector < 0) return vector; apic_update_vector(irqd, vector, cpu); - apic_update_irq_cfg(irqd, vector, cpu); return 0; } @@ -357,7 +364,7 @@ static void clear_irq_vector(struct irq_ apicd->prev_cpu); per_cpu(vector_irq, apicd->cpu)[vector] = VECTOR_SHUTDOWN; - irq_matrix_free(vector_matrix, apicd->cpu, vector, managed); + apic_free_vector(apicd->cpu, vector, managed); apicd->vector = 0; /* Clean up move in progress */ @@ -366,7 +373,7 @@ static void clear_irq_vector(struct irq_ return; per_cpu(vector_irq, apicd->prev_cpu)[vector] = VECTOR_SHUTDOWN; - irq_matrix_free(vector_matrix, apicd->prev_cpu, vector, managed); + apic_free_vector(apicd->prev_cpu, vector, managed); apicd->prev_vector = 0; apicd->move_in_progress = 0; hlist_del_init(&apicd->clist); @@ -905,7 +912,7 @@ static void free_moved_vector(struct api * affinity mask comes online. */ trace_vector_free_moved(apicd->irq, cpu, vector, managed); - irq_matrix_free(vector_matrix, cpu, vector, managed); + apic_free_vector(cpu, vector, managed); per_cpu(vector_irq, cpu)[vector] = VECTOR_UNUSED; hlist_del_init(&apicd->clist); apicd->prev_vector = 0;
On Thu, Apr 17 2025 at 14:46, Neeraj Upadhyay wrote: > + > +static inline void update_vector(unsigned int cpu, unsigned int offset, > + unsigned int vector, bool set) > +{ > + unsigned long *reg = get_reg_bitmap(cpu, offset); > + unsigned int bit = get_vec_bit(vector); > + > + if (set) > + set_bit(bit, reg); > + else > + clear_bit(bit, reg); > +} > +static void savic_update_vector(unsigned int cpu, unsigned int vector, bool set) > +{ > + update_vector(cpu, SAVIC_ALLOWED_IRR, vector, set); This indirection is required because otherwise the code is too simple to follow?
On 4/17/2025 4:20 PM, Thomas Gleixner wrote: > On Thu, Apr 17 2025 at 14:46, Neeraj Upadhyay wrote: >> Add update_vector callback to set/clear ALLOWED_IRR field in >> a vCPU's APIC backing page for external vectors. The ALLOWED_IRR >> field indicates the interrupt vectors which the guest allows the >> hypervisor to send (typically for emulated devices). Interrupt >> vectors used exclusively by the guest itself and the vectors which >> are not emulated by the hypervisor, such as IPI vectors, are part >> of system vectors and are not set in the ALLOWED_IRR. > > Please structure changelogs properly in paragraphs: > > https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#changelog > Ok >> arch/x86/include/asm/apic.h | 9 +++++ >> arch/x86/kernel/apic/vector.c | 53 ++++++++++++++++++++++------- >> arch/x86/kernel/apic/x2apic_savic.c | 35 +++++++++++++++++++ > > And split this patch up into two: > > 1) Do the modifications in vector.c which is what the $Subject line > says > > 2) Add the SAVIC specific bits > Ok >> @@ -471,6 +473,12 @@ static __always_inline bool apic_id_valid(u32 apic_id) >> return apic_id <= apic->max_apic_id; >> } >> >> +static __always_inline void apic_update_vector(unsigned int cpu, unsigned int vector, bool set) >> +{ >> + if (apic->update_vector) >> + apic->update_vector(cpu, vector, set); >> +} > > This is in the public header because it can? > apic_update_vector() is needed for some system vectors which are emulated/injected by Hypervisor. Patch 8 calls it for lapic timer. HYPERVISOR_CALLBACK_VECTOR would need it for hyperv. This patch series does not call apic_update_vector() for HYPERVISOR_CALLBACK_VECTOR though. Given that currently this callback is not used outside of apic code, do I need to add it to arch/x86/kernel/apic/local.h or just remove it and use conditional call in current callsites? >> -static void apic_update_vector(struct irq_data *irqd, unsigned int newvec, >> - unsigned int newcpu) >> +static int irq_alloc_vector(const struct cpumask *dest, bool resvd, unsigned int *cpu) >> +{ >> + int vector; >> + >> + vector = irq_matrix_alloc(vector_matrix, dest, resvd, cpu); > > int vector = irq_matrix_alloc(...); > Ok >> + >> + if (vector >= 0) >> + apic_update_vector(*cpu, vector, true); >> + >> + return vector; >> +} >> + >> +static int irq_alloc_managed_vector(unsigned int *cpu) >> +{ >> + int vector; >> + >> + vector = irq_matrix_alloc_managed(vector_matrix, vector_searchmask, cpu); >> + >> + if (vector >= 0) >> + apic_update_vector(*cpu, vector, true); >> + >> + return vector; >> +} > > I completely fail to see the value of these two functions. Each of them > has exactly _ONE_ call site and both sites invoke apic_update_vector() Ok, this was done to associate apic->update_vector() calls with the setup (irq_matrix_alloc()) and teardown (irq_matrix_free()) of vector, which was my understanding from your suggestion here: https://lore.kernel.org/lkml/87jz8i31dv.ffs@tglx/ . Given that there is single callsite for each and vector_configure_legacy() calls apic->update_vector() outside of alloc path, adding it to apic_update_irq_cfg(), as you suggest handles all callers. > when the allocation succeeded. Why can't you just do the obvious and > leave the existing code alone and add > > if (apic->update_vector) > apic->update_vector(); > > into apic_update_vector()? But then you have another place where you > need the update, which does not invoke apic_update_vector(). > > Now if you look deeper, then you notice that all places which invoke > apic_update_vector() invoke apic_update_irq_cfg(), which is also called > at this other place, no? > Yes, makes sense. apic_update_irq_cfg() is called for MANAGED_IRQ_SHUTDOWN_VECTOR also. I am not aware of the use case of that vector. Whethere it is an interrupt which is injected by Hypervisor. static void vector_assign_managed_shutdown(struct irq_data *irqd) { unsigned int cpu = cpumask_first(cpu_online_mask); apic_update_irq_cfg(irqd, MANAGED_IRQ_SHUTDOWN_VECTOR, cpu); } >> +static void irq_free_vector(unsigned int cpu, unsigned int vector, bool managed) >> +{ >> + apic_update_vector(cpu, vector, false); >> + irq_matrix_free(vector_matrix, cpu, vector, managed); >> +} > > This one makes sense, but please name it: apic_free_vector() > Ok > Something like the uncompiled below, no? > Ok, makes sense. Looks good. - Neeraj > Thanks, > > tglx > --- > --- a/arch/x86/kernel/apic/vector.c > +++ b/arch/x86/kernel/apic/vector.c > @@ -134,9 +134,19 @@ static void apic_update_irq_cfg(struct i > > apicd->hw_irq_cfg.vector = vector; > apicd->hw_irq_cfg.dest_apicid = apic->calc_dest_apicid(cpu); > + > + if (apic->update_vector) > + apic->update_vector(cpu, vector, true); > + > irq_data_update_effective_affinity(irqd, cpumask_of(cpu)); > - trace_vector_config(irqd->irq, vector, cpu, > - apicd->hw_irq_cfg.dest_apicid); > + trace_vector_config(irqd->irq, vector, cpu, apicd->hw_irq_cfg.dest_apicid); > +} > + > +static void apic_free_vector(unsigned int cpu, unsigned int vector, bool managed) > +{ > + if (apic->update_vector) > + apic->update_vector(cpu, vector, false); > + irq_matrix_free(vector_matrix, cpu, vector, managed); > } > > static void apic_update_vector(struct irq_data *irqd, unsigned int newvec, > @@ -174,8 +184,7 @@ static void apic_update_vector(struct ir > apicd->prev_cpu = apicd->cpu; > WARN_ON_ONCE(apicd->cpu == newcpu); > } else { > - irq_matrix_free(vector_matrix, apicd->cpu, apicd->vector, > - managed); > + apic_free_vector(apicd->cpu, apicd->vector, managed); > } > > setnew: > @@ -183,6 +192,7 @@ static void apic_update_vector(struct ir > apicd->cpu = newcpu; > BUG_ON(!IS_ERR_OR_NULL(per_cpu(vector_irq, newcpu)[newvec])); > per_cpu(vector_irq, newcpu)[newvec] = desc; > + apic_update_irq_cfg(irqd, newvec, newcpu); > } > > static void vector_assign_managed_shutdown(struct irq_data *irqd) > @@ -261,8 +271,6 @@ assign_vector_locked(struct irq_data *ir > if (vector < 0) > return vector; > apic_update_vector(irqd, vector, cpu); > - apic_update_irq_cfg(irqd, vector, cpu); > - > return 0; > } > > @@ -338,7 +346,6 @@ assign_managed_vector(struct irq_data *i > if (vector < 0) > return vector; > apic_update_vector(irqd, vector, cpu); > - apic_update_irq_cfg(irqd, vector, cpu); > return 0; > } > > @@ -357,7 +364,7 @@ static void clear_irq_vector(struct irq_ > apicd->prev_cpu); > > per_cpu(vector_irq, apicd->cpu)[vector] = VECTOR_SHUTDOWN; > - irq_matrix_free(vector_matrix, apicd->cpu, vector, managed); > + apic_free_vector(apicd->cpu, vector, managed); > apicd->vector = 0; > > /* Clean up move in progress */ > @@ -366,7 +373,7 @@ static void clear_irq_vector(struct irq_ > return; > > per_cpu(vector_irq, apicd->prev_cpu)[vector] = VECTOR_SHUTDOWN; > - irq_matrix_free(vector_matrix, apicd->prev_cpu, vector, managed); > + apic_free_vector(apicd->prev_cpu, vector, managed); > apicd->prev_vector = 0; > apicd->move_in_progress = 0; > hlist_del_init(&apicd->clist); > @@ -905,7 +912,7 @@ static void free_moved_vector(struct api > * affinity mask comes online. > */ > trace_vector_free_moved(apicd->irq, cpu, vector, managed); > - irq_matrix_free(vector_matrix, cpu, vector, managed); > + apic_free_vector(cpu, vector, managed); > per_cpu(vector_irq, cpu)[vector] = VECTOR_UNUSED; > hlist_del_init(&apicd->clist); > apicd->prev_vector = 0;
On 4/17/2025 4:22 PM, Thomas Gleixner wrote: > On Thu, Apr 17 2025 at 14:46, Neeraj Upadhyay wrote: >> + >> +static inline void update_vector(unsigned int cpu, unsigned int offset, >> + unsigned int vector, bool set) >> +{ >> + unsigned long *reg = get_reg_bitmap(cpu, offset); >> + unsigned int bit = get_vec_bit(vector); >> + >> + if (set) >> + set_bit(bit, reg); >> + else >> + clear_bit(bit, reg); >> +} > >> +static void savic_update_vector(unsigned int cpu, unsigned int vector, bool set) >> +{ >> + update_vector(cpu, SAVIC_ALLOWED_IRR, vector, set); > > This indirection is required because otherwise the code is too simple to > follow? > update_vector() is used by send_ipi_dest() in Patch 7. From your comment on v3 https://lore.kernel.org/lkml/87y0whv57k.ffs@tglx/ , what I understood was that you wanted update_vector() to be defined in the patch where that code is added (i.e. this patch) and not at a later patch. Is that not correct understanding? - Neeraj
On Thu, Apr 17 2025 at 17:30, Neeraj Upadhyay wrote: > On 4/17/2025 4:20 PM, Thomas Gleixner wrote: >>> +static __always_inline void apic_update_vector(unsigned int cpu, unsigned int vector, bool set) >>> +{ >>> + if (apic->update_vector) >>> + apic->update_vector(cpu, vector, set); >>> +} >> >> This is in the public header because it can? >> > > apic_update_vector() is needed for some system vectors which are emulated/injected > by Hypervisor. Patch 8 calls it for lapic timer. HYPERVISOR_CALLBACK_VECTOR would need > it for hyperv. This patch series does not call apic_update_vector() for > HYPERVISOR_CALLBACK_VECTOR though. Then explain so in the change log instead of letting reviewers wonder. I'm patently bad at reading your thoughts. >> Now if you look deeper, then you notice that all places which invoke >> apic_update_vector() invoke apic_update_irq_cfg(), which is also called >> at this other place, no? >> > > Yes, makes sense. apic_update_irq_cfg() is called for MANAGED_IRQ_SHUTDOWN_VECTOR > also. I am not aware of the use case of that vector. Whethere it is an interrupt > which is injected by Hypervisor. It can. The managed shutdown vector is used when a queue of a managed multiqueue device is shut down. The device MSI-X entry is redirected to this special vector in case that the device/hardware raises an unexpected spurious interrupt post shut down. Thanks, tglx
On Thu, Apr 17 2025 at 17:42, Neeraj Upadhyay wrote: > On 4/17/2025 4:22 PM, Thomas Gleixner wrote: >>> +static void savic_update_vector(unsigned int cpu, unsigned int vector, bool set) >>> +{ >>> + update_vector(cpu, SAVIC_ALLOWED_IRR, vector, set); >> >> This indirection is required because otherwise the code is too simple to >> follow? >> > update_vector() is used by send_ipi_dest() in Patch 7. From your comment > on v3 https://lore.kernel.org/lkml/87y0whv57k.ffs@tglx/ , what I understood > was that you wanted update_vector() to be defined in the patch where that code > is added (i.e. this patch) and not at a later patch. Is that not correct > understanding? Fair enough. I missed the later usage sites. Again, a short note in the change log which explains the rationale would avoid this. Thanks, tglx
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h index 562115100038..c359cce60b22 100644 --- a/arch/x86/include/asm/apic.h +++ b/arch/x86/include/asm/apic.h @@ -318,6 +318,8 @@ struct apic { /* wakeup secondary CPU using 64-bit wakeup point */ int (*wakeup_secondary_cpu_64)(u32 apicid, unsigned long start_eip); + void (*update_vector)(unsigned int cpu, unsigned int vector, bool set); + char *name; }; @@ -471,6 +473,12 @@ static __always_inline bool apic_id_valid(u32 apic_id) return apic_id <= apic->max_apic_id; } +static __always_inline void apic_update_vector(unsigned int cpu, unsigned int vector, bool set) +{ + if (apic->update_vector) + apic->update_vector(cpu, vector, set); +} + #else /* CONFIG_X86_LOCAL_APIC */ static inline u32 apic_read(u32 reg) { return 0; } @@ -482,6 +490,7 @@ static inline void apic_wait_icr_idle(void) { } static inline u32 safe_apic_wait_icr_idle(void) { return 0; } static inline void apic_native_eoi(void) { WARN_ON_ONCE(1); } static inline void apic_setup_apic_calls(void) { } +static inline void apic_update_vector(unsigned int cpu, unsigned int vector, bool set) { } #define apic_update_callback(_callback, _fn) do { } while (0) diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c index fee42a73d64a..351db49b9975 100644 --- a/arch/x86/kernel/apic/vector.c +++ b/arch/x86/kernel/apic/vector.c @@ -139,8 +139,38 @@ static void apic_update_irq_cfg(struct irq_data *irqd, unsigned int vector, apicd->hw_irq_cfg.dest_apicid); } -static void apic_update_vector(struct irq_data *irqd, unsigned int newvec, - unsigned int newcpu) +static int irq_alloc_vector(const struct cpumask *dest, bool resvd, unsigned int *cpu) +{ + int vector; + + vector = irq_matrix_alloc(vector_matrix, dest, resvd, cpu); + + if (vector >= 0) + apic_update_vector(*cpu, vector, true); + + return vector; +} + +static int irq_alloc_managed_vector(unsigned int *cpu) +{ + int vector; + + vector = irq_matrix_alloc_managed(vector_matrix, vector_searchmask, cpu); + + if (vector >= 0) + apic_update_vector(*cpu, vector, true); + + return vector; +} + +static void irq_free_vector(unsigned int cpu, unsigned int vector, bool managed) +{ + apic_update_vector(cpu, vector, false); + irq_matrix_free(vector_matrix, cpu, vector, managed); +} + +static void apic_chipd_update_vector(struct irq_data *irqd, unsigned int newvec, + unsigned int newcpu) { struct apic_chip_data *apicd = apic_chip_data(irqd); struct irq_desc *desc = irq_data_to_desc(irqd); @@ -174,8 +204,7 @@ static void apic_update_vector(struct irq_data *irqd, unsigned int newvec, apicd->prev_cpu = apicd->cpu; WARN_ON_ONCE(apicd->cpu == newcpu); } else { - irq_matrix_free(vector_matrix, apicd->cpu, apicd->vector, - managed); + irq_free_vector(apicd->cpu, apicd->vector, managed); } setnew: @@ -256,11 +285,11 @@ assign_vector_locked(struct irq_data *irqd, const struct cpumask *dest) if (apicd->move_in_progress || !hlist_unhashed(&apicd->clist)) return -EBUSY; - vector = irq_matrix_alloc(vector_matrix, dest, resvd, &cpu); + vector = irq_alloc_vector(dest, resvd, &cpu); trace_vector_alloc(irqd->irq, vector, resvd, vector); if (vector < 0) return vector; - apic_update_vector(irqd, vector, cpu); + apic_chipd_update_vector(irqd, vector, cpu); apic_update_irq_cfg(irqd, vector, cpu); return 0; @@ -332,12 +361,11 @@ assign_managed_vector(struct irq_data *irqd, const struct cpumask *dest) /* set_affinity might call here for nothing */ if (apicd->vector && cpumask_test_cpu(apicd->cpu, vector_searchmask)) return 0; - vector = irq_matrix_alloc_managed(vector_matrix, vector_searchmask, - &cpu); + vector = irq_alloc_managed_vector(&cpu); trace_vector_alloc_managed(irqd->irq, vector, vector); if (vector < 0) return vector; - apic_update_vector(irqd, vector, cpu); + apic_chipd_update_vector(irqd, vector, cpu); apic_update_irq_cfg(irqd, vector, cpu); return 0; } @@ -357,7 +385,7 @@ static void clear_irq_vector(struct irq_data *irqd) apicd->prev_cpu); per_cpu(vector_irq, apicd->cpu)[vector] = VECTOR_SHUTDOWN; - irq_matrix_free(vector_matrix, apicd->cpu, vector, managed); + irq_free_vector(apicd->cpu, vector, managed); apicd->vector = 0; /* Clean up move in progress */ @@ -366,7 +394,7 @@ static void clear_irq_vector(struct irq_data *irqd) return; per_cpu(vector_irq, apicd->prev_cpu)[vector] = VECTOR_SHUTDOWN; - irq_matrix_free(vector_matrix, apicd->prev_cpu, vector, managed); + irq_free_vector(apicd->prev_cpu, vector, managed); apicd->prev_vector = 0; apicd->move_in_progress = 0; hlist_del_init(&apicd->clist); @@ -528,6 +556,7 @@ static bool vector_configure_legacy(unsigned int virq, struct irq_data *irqd, if (irqd_is_activated(irqd)) { trace_vector_setup(virq, true, 0); apic_update_irq_cfg(irqd, apicd->vector, apicd->cpu); + apic_update_vector(apicd->cpu, apicd->vector, true); } else { /* Release the vector */ apicd->can_reserve = true; @@ -905,7 +934,7 @@ static void free_moved_vector(struct apic_chip_data *apicd) * affinity mask comes online. */ trace_vector_free_moved(apicd->irq, cpu, vector, managed); - irq_matrix_free(vector_matrix, cpu, vector, managed); + irq_free_vector(cpu, vector, managed); per_cpu(vector_irq, cpu)[vector] = VECTOR_UNUSED; hlist_del_init(&apicd->clist); apicd->prev_vector = 0; diff --git a/arch/x86/kernel/apic/x2apic_savic.c b/arch/x86/kernel/apic/x2apic_savic.c index 81d932061b7b..9d2e93656037 100644 --- a/arch/x86/kernel/apic/x2apic_savic.c +++ b/arch/x86/kernel/apic/x2apic_savic.c @@ -43,6 +43,34 @@ static __always_inline void set_reg(unsigned int offset, u32 val) WRITE_ONCE(this_cpu_ptr(apic_page)->regs[offset >> 2], val); } +static inline unsigned long *get_reg_bitmap(unsigned int cpu, unsigned int offset) +{ + struct apic_page *ap = per_cpu_ptr(apic_page, cpu); + + return (unsigned long *) &ap->bytes[offset]; +} + +static inline unsigned int get_vec_bit(unsigned int vector) +{ + /* + * The registers are 32-bit wide and 16-byte aligned. + * Compensate for the resulting bit number spacing. + */ + return vector + 96 * (vector / 32); +} + +static inline void update_vector(unsigned int cpu, unsigned int offset, + unsigned int vector, bool set) +{ + unsigned long *reg = get_reg_bitmap(cpu, offset); + unsigned int bit = get_vec_bit(vector); + + if (set) + set_bit(bit, reg); + else + clear_bit(bit, reg); +} + #define SAVIC_ALLOWED_IRR 0x204 static u32 savic_read(u32 reg) @@ -144,6 +172,11 @@ static void savic_write(u32 reg, u32 data) } } +static void savic_update_vector(unsigned int cpu, unsigned int vector, bool set) +{ + update_vector(cpu, SAVIC_ALLOWED_IRR, vector, set); +} + static void init_apic_page(void) { u32 apic_id; @@ -225,6 +258,8 @@ static struct apic apic_x2apic_savic __ro_after_init = { .eoi = native_apic_msr_eoi, .icr_read = native_x2apic_icr_read, .icr_write = native_x2apic_icr_write, + + .update_vector = savic_update_vector, }; apic_driver(apic_x2apic_savic);