[PATCH v9 05/18] x86/apic: Add update_vector() callback for apic drivers

Neeraj Upadhyay posted 18 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v9 05/18] x86/apic: Add update_vector() callback for apic drivers
Posted by Neeraj Upadhyay 1 month, 3 weeks ago
Add an update_vector() callback to allow APIC drivers to perform
driver specific operations on external vector allocation/teardown
on a CPU. This callback will be used in subsequent commits by Secure
AVIC APIC driver to configure the vectors which a guest vCPU allows
the hypervisor to send to it.

As system vectors have fixed vector assignments and are not dynamically
allocated, add apic_update_vector() public api to facilitate
update_vector() callback invocation for them. This will be used for
Secure AVIC enabled guests to allow the hypervisor to inject system
vectors which are emulated by the hypervisor such as APIC timer vector
and HYPERVISOR_CALLBACK_VECTOR.

While at it, cleanup line break in apic_update_irq_cfg().

Co-developed-by: Kishon Vijay Abraham I <kvijayab@amd.com>
Signed-off-by: Kishon Vijay Abraham I <kvijayab@amd.com>
Signed-off-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
---
Changes since v8:
 - No change.

 arch/x86/include/asm/apic.h   |  9 +++++++++
 arch/x86/kernel/apic/vector.c | 29 ++++++++++++++++++-----------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 44b4080721a6..0683318470be 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, unsigned int cpu);
 
+	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 a947b46a8b64..655eeb808ebc 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -134,13 +134,21 @@ static void apic_update_irq_cfg(struct irq_data *irqd, unsigned int vector,
 
 	apicd->hw_irq_cfg.vector = vector;
 	apicd->hw_irq_cfg.dest_apicid = apic->calc_dest_apicid(cpu);
+
+	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_update_vector(struct irq_data *irqd, unsigned int newvec,
-			       unsigned int newcpu)
+static void apic_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 +182,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);
+		apic_free_vector(apicd->cpu, apicd->vector, managed);
 	}
 
 setnew:
@@ -261,7 +268,7 @@ assign_vector_locked(struct irq_data *irqd, const struct cpumask *dest)
 	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);
 
 	return 0;
 }
@@ -337,7 +344,7 @@ assign_managed_vector(struct irq_data *irqd, const struct cpumask *dest)
 	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);
 
 	return 0;
 }
@@ -357,7 +364,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);
+	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_data *irqd)
 		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 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);
+	apic_free_vector(cpu, vector, managed);
 	per_cpu(vector_irq, cpu)[vector] = VECTOR_UNUSED;
 	hlist_del_init(&apicd->clist);
 	apicd->prev_vector = 0;
-- 
2.34.1
Re: [PATCH v9 05/18] x86/apic: Add update_vector() callback for apic drivers
Posted by Borislav Petkov 1 month, 2 weeks ago
On Mon, Aug 11, 2025 at 03:14:31PM +0530, Neeraj Upadhyay wrote:
> +static void apic_chipd_update_vector(struct irq_data *irqd, unsigned int newvec,

What is "chipd" supposed to denote?

> +				     unsigned int newcpu)
>  {
>  	struct apic_chip_data *apicd = apic_chip_data(irqd);
>  	struct irq_desc *desc = irq_data_to_desc(irqd);

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v9 05/18] x86/apic: Add update_vector() callback for apic drivers
Posted by Upadhyay, Neeraj 1 month, 2 weeks ago

On 8/20/2025 3:29 AM, Borislav Petkov wrote:
> On Mon, Aug 11, 2025 at 03:14:31PM +0530, Neeraj Upadhyay wrote:
>> +static void apic_chipd_update_vector(struct irq_data *irqd, unsigned int newvec,
> 
> What is "chipd" supposed to denote?
> 

chip data (struct apic_chip_data)


- Neeraj

>> +				     unsigned int newcpu)
>>   {
>>   	struct apic_chip_data *apicd = apic_chip_data(irqd);
>>   	struct irq_desc *desc = irq_data_to_desc(irqd);
>
Re: [PATCH v9 05/18] x86/apic: Add update_vector() callback for apic drivers
Posted by Borislav Petkov 1 month, 1 week ago
On Wed, Aug 20, 2025 at 09:06:52AM +0530, Upadhyay, Neeraj wrote:
> > > +static void apic_chipd_update_vector(struct irq_data *irqd, unsigned int newvec,
> > 
> > What is "chipd" supposed to denote?
> 
> chip data (struct apic_chip_data)

So this function should be called chip_data_update() or so?

It is static so it doesn't need the "apic_" prefix and "chipd" doesn't make
a whole lotta of sense so let's call it as what it does.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v9 05/18] x86/apic: Add update_vector() callback for apic drivers
Posted by Upadhyay, Neeraj 1 month, 1 week ago

On 8/25/2025 8:19 PM, Borislav Petkov wrote:
> On Wed, Aug 20, 2025 at 09:06:52AM +0530, Upadhyay, Neeraj wrote:
>>>> +static void apic_chipd_update_vector(struct irq_data *irqd, unsigned int newvec,
>>>
>>> What is "chipd" supposed to denote?
>>
>> chip data (struct apic_chip_data)
> 
> So this function should be called chip_data_update() or so?
> 

or chip_data_update_vector() as the updates are specific for a new 
vector assignment?

> It is static so it doesn't need the "apic_" prefix and "chipd" doesn't make
> a whole lotta of sense so let's call it as what it does.
> 

Got it. However, I see other static functions in this file using "apic_" 
prefix (in some cases, maybe to differentiate "apic_chip_data" from a 
generic "chip_data" in common kernel/irq/ subys?).


- Neeraj
Re: [PATCH v9 05/18] x86/apic: Add update_vector() callback for apic drivers
Posted by Borislav Petkov 1 month, 1 week ago
On Tue, Aug 26, 2025 at 09:36:56AM +0530, Upadhyay, Neeraj wrote:
> or chip_data_update_vector() as the updates are specific for a new vector
> assignment?

This function assigns a bunch of things to struct apic_chip_data - not only
vector.

If we had to be precise perhaps update_vector_chip_data() or so. Or
chip_data_update_for_vector(). To better describe what the function does.

> Got it. However, I see other static functions in this file using "apic_"
> prefix (in some cases, maybe to differentiate "apic_chip_data" from a
> generic "chip_data" in common kernel/irq/ subys?).

Looks like this file would need cleaning up wrt naming but that's not that
important at the moment I'd say. There are other functions which don't have
the apic_ prefix so it is kinda arbitrary.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette