To prepare native usage of posted interrupt, move PID declaration out of
VMX code such that they can be shared.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
arch/x86/include/asm/posted_intr.h | 97 ++++++++++++++++++++++++++++++
arch/x86/kvm/vmx/posted_intr.h | 93 +---------------------------
arch/x86/kvm/vmx/vmx.c | 1 +
arch/x86/kvm/vmx/vmx.h | 2 +-
4 files changed, 100 insertions(+), 93 deletions(-)
create mode 100644 arch/x86/include/asm/posted_intr.h
diff --git a/arch/x86/include/asm/posted_intr.h b/arch/x86/include/asm/posted_intr.h
new file mode 100644
index 000000000000..9f2fa38fa57b
--- /dev/null
+++ b/arch/x86/include/asm/posted_intr.h
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _X86_POSTED_INTR_H
+#define _X86_POSTED_INTR_H
+
+#define POSTED_INTR_ON 0
+#define POSTED_INTR_SN 1
+
+#define PID_TABLE_ENTRY_VALID 1
+
+/* Posted-Interrupt Descriptor */
+struct pi_desc {
+ u32 pir[8]; /* Posted interrupt requested */
+ union {
+ struct {
+ /* bit 256 - Outstanding Notification */
+ u16 on : 1,
+ /* bit 257 - Suppress Notification */
+ sn : 1,
+ /* bit 271:258 - Reserved */
+ rsvd_1 : 14;
+ /* bit 279:272 - Notification Vector */
+ u8 nv;
+ /* bit 287:280 - Reserved */
+ u8 rsvd_2;
+ /* bit 319:288 - Notification Destination */
+ u32 ndst;
+ };
+ u64 control;
+ };
+ u32 rsvd[6];
+} __aligned(64);
+
+static inline bool pi_test_and_set_on(struct pi_desc *pi_desc)
+{
+ return test_and_set_bit(POSTED_INTR_ON,
+ (unsigned long *)&pi_desc->control);
+}
+
+static inline bool pi_test_and_clear_on(struct pi_desc *pi_desc)
+{
+ return test_and_clear_bit(POSTED_INTR_ON,
+ (unsigned long *)&pi_desc->control);
+}
+
+static inline bool pi_test_and_clear_sn(struct pi_desc *pi_desc)
+{
+ return test_and_clear_bit(POSTED_INTR_SN,
+ (unsigned long *)&pi_desc->control);
+}
+
+static inline bool pi_test_and_set_pir(int vector, struct pi_desc *pi_desc)
+{
+ return test_and_set_bit(vector, (unsigned long *)pi_desc->pir);
+}
+
+static inline bool pi_is_pir_empty(struct pi_desc *pi_desc)
+{
+ return bitmap_empty((unsigned long *)pi_desc->pir, NR_VECTORS);
+}
+
+static inline void pi_set_sn(struct pi_desc *pi_desc)
+{
+ set_bit(POSTED_INTR_SN,
+ (unsigned long *)&pi_desc->control);
+}
+
+static inline void pi_set_on(struct pi_desc *pi_desc)
+{
+ set_bit(POSTED_INTR_ON,
+ (unsigned long *)&pi_desc->control);
+}
+
+static inline void pi_clear_on(struct pi_desc *pi_desc)
+{
+ clear_bit(POSTED_INTR_ON,
+ (unsigned long *)&pi_desc->control);
+}
+
+static inline void pi_clear_sn(struct pi_desc *pi_desc)
+{
+ clear_bit(POSTED_INTR_SN,
+ (unsigned long *)&pi_desc->control);
+}
+
+static inline bool pi_test_on(struct pi_desc *pi_desc)
+{
+ return test_bit(POSTED_INTR_ON,
+ (unsigned long *)&pi_desc->control);
+}
+
+static inline bool pi_test_sn(struct pi_desc *pi_desc)
+{
+ return test_bit(POSTED_INTR_SN,
+ (unsigned long *)&pi_desc->control);
+}
+
+#endif /* _X86_POSTED_INTR_H */
diff --git a/arch/x86/kvm/vmx/posted_intr.h b/arch/x86/kvm/vmx/posted_intr.h
index 26992076552e..6b2a0226257e 100644
--- a/arch/x86/kvm/vmx/posted_intr.h
+++ b/arch/x86/kvm/vmx/posted_intr.h
@@ -1,98 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
#ifndef __KVM_X86_VMX_POSTED_INTR_H
#define __KVM_X86_VMX_POSTED_INTR_H
-
-#define POSTED_INTR_ON 0
-#define POSTED_INTR_SN 1
-
-#define PID_TABLE_ENTRY_VALID 1
-
-/* Posted-Interrupt Descriptor */
-struct pi_desc {
- u32 pir[8]; /* Posted interrupt requested */
- union {
- struct {
- /* bit 256 - Outstanding Notification */
- u16 on : 1,
- /* bit 257 - Suppress Notification */
- sn : 1,
- /* bit 271:258 - Reserved */
- rsvd_1 : 14;
- /* bit 279:272 - Notification Vector */
- u8 nv;
- /* bit 287:280 - Reserved */
- u8 rsvd_2;
- /* bit 319:288 - Notification Destination */
- u32 ndst;
- };
- u64 control;
- };
- u32 rsvd[6];
-} __aligned(64);
-
-static inline bool pi_test_and_set_on(struct pi_desc *pi_desc)
-{
- return test_and_set_bit(POSTED_INTR_ON,
- (unsigned long *)&pi_desc->control);
-}
-
-static inline bool pi_test_and_clear_on(struct pi_desc *pi_desc)
-{
- return test_and_clear_bit(POSTED_INTR_ON,
- (unsigned long *)&pi_desc->control);
-}
-
-static inline bool pi_test_and_clear_sn(struct pi_desc *pi_desc)
-{
- return test_and_clear_bit(POSTED_INTR_SN,
- (unsigned long *)&pi_desc->control);
-}
-
-static inline bool pi_test_and_set_pir(int vector, struct pi_desc *pi_desc)
-{
- return test_and_set_bit(vector, (unsigned long *)pi_desc->pir);
-}
-
-static inline bool pi_is_pir_empty(struct pi_desc *pi_desc)
-{
- return bitmap_empty((unsigned long *)pi_desc->pir, NR_VECTORS);
-}
-
-static inline void pi_set_sn(struct pi_desc *pi_desc)
-{
- set_bit(POSTED_INTR_SN,
- (unsigned long *)&pi_desc->control);
-}
-
-static inline void pi_set_on(struct pi_desc *pi_desc)
-{
- set_bit(POSTED_INTR_ON,
- (unsigned long *)&pi_desc->control);
-}
-
-static inline void pi_clear_on(struct pi_desc *pi_desc)
-{
- clear_bit(POSTED_INTR_ON,
- (unsigned long *)&pi_desc->control);
-}
-
-static inline void pi_clear_sn(struct pi_desc *pi_desc)
-{
- clear_bit(POSTED_INTR_SN,
- (unsigned long *)&pi_desc->control);
-}
-
-static inline bool pi_test_on(struct pi_desc *pi_desc)
-{
- return test_bit(POSTED_INTR_ON,
- (unsigned long *)&pi_desc->control);
-}
-
-static inline bool pi_test_sn(struct pi_desc *pi_desc)
-{
- return test_bit(POSTED_INTR_SN,
- (unsigned long *)&pi_desc->control);
-}
+#include <asm/posted_intr.h>
void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu);
void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 72e3943f3693..d54fa0e06c70 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -66,6 +66,7 @@
#include "vmx.h"
#include "x86.h"
#include "smm.h"
+#include "posted_intr.h"
MODULE_AUTHOR("Qumranet");
MODULE_LICENSE("GPL");
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index c2130d2c8e24..817b76794ee1 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -7,10 +7,10 @@
#include <asm/kvm.h>
#include <asm/intel_pt.h>
#include <asm/perf_event.h>
+#include <asm/posted_intr.h>
#include "capabilities.h"
#include "../kvm_cache_regs.h"
-#include "posted_intr.h"
#include "vmcs.h"
#include "vmx_ops.h"
#include "../cpuid.h"
--
2.25.1On Sat, Nov 11 2023 at 20:16, Jacob Pan wrote:
> +/* Posted-Interrupt Descriptor */
> +struct pi_desc {
> + u32 pir[8]; /* Posted interrupt requested */
> + union {
> + struct {
> + /* bit 256 - Outstanding Notification */
> + u16 on : 1,
> + /* bit 257 - Suppress Notification */
> + sn : 1,
> + /* bit 271:258 - Reserved */
> + rsvd_1 : 14;
> + /* bit 279:272 - Notification Vector */
> + u8 nv;
> + /* bit 287:280 - Reserved */
> + u8 rsvd_2;
> + /* bit 319:288 - Notification Destination */
> + u32 ndst;
This mixture of bitfields and types is weird and really not intuitive:
/* Posted-Interrupt Descriptor */
struct pi_desc {
/* Posted interrupt requested */
u32 pir[8];
union {
struct {
/* bit 256 - Outstanding Notification */
u64 on : 1,
/* bit 257 - Suppress Notification */
sn : 1,
/* bit 271:258 - Reserved */
: 14,
/* bit 279:272 - Notification Vector */
nv : 8,
/* bit 287:280 - Reserved */
: 8,
/* bit 319:288 - Notification Destination */
ndst : 32;
};
u64 control;
};
u32 rsvd[6];
} __aligned(64);
Hmm?
> +static inline bool pi_test_and_set_on(struct pi_desc *pi_desc)
> +{
> + return test_and_set_bit(POSTED_INTR_ON,
> + (unsigned long *)&pi_desc->control);
Please get rid of those line breaks.
Thanks,
tglx
Hi Thomas,
On Wed, 06 Dec 2023 17:33:28 +0100, Thomas Gleixner <tglx@linutronix.de>
wrote:
> On Sat, Nov 11 2023 at 20:16, Jacob Pan wrote:
> > +/* Posted-Interrupt Descriptor */
> > +struct pi_desc {
> > + u32 pir[8]; /* Posted interrupt requested */
> > + union {
> > + struct {
> > + /* bit 256 - Outstanding Notification
> > */
> > + u16 on : 1,
> > + /* bit 257 - Suppress Notification */
> > + sn : 1,
> > + /* bit 271:258 - Reserved */
> > + rsvd_1 : 14;
> > + /* bit 279:272 - Notification Vector */
> > + u8 nv;
> > + /* bit 287:280 - Reserved */
> > + u8 rsvd_2;
> > + /* bit 319:288 - Notification
> > Destination */
> > + u32 ndst;
>
> This mixture of bitfields and types is weird and really not intuitive:
>
> /* Posted-Interrupt Descriptor */
> struct pi_desc {
> /* Posted interrupt requested */
> u32 pir[8];
>
> union {
> struct {
> /* bit 256 - Outstanding Notification */
> u64 on : 1,
> /* bit 257 - Suppress Notification */
> sn : 1,
> /* bit 271:258 - Reserved */
> : 14,
> /* bit 279:272 - Notification Vector */
> nv : 8,
> /* bit 287:280 - Reserved */
> : 8,
> /* bit 319:288 - Notification Destination
> */ ndst : 32;
> };
> u64 control;
> };
> u32 rsvd[6];
> } __aligned(64);
>
It seems bit-fields cannot pass type check. I got these compile error.
arch/x86/kernel/irq.c: In function ‘intel_posted_msi_init’:
./include/linux/percpu-defs.h:363:20: error: cannot take address of bit-field ‘nv’
363 | __verify_pcpu_ptr(&(variable)); \
| ^
./include/linux/percpu-defs.h:219:47: note: in definition of macro ‘__verify_pcpu_ptr’
219 | const void __percpu *__vpp_verify = (typeof((ptr) + 0))NULL; \
| ^~~
./include/linux/percpu-defs.h:490:34: note: in expansion of macro ‘__pcpu_size_call’
490 | #define this_cpu_write(pcp, val) __pcpu_size_call(this_cpu_write_, pcp, val)
| ^~~~~~~~~~~~~~~~
arch/x86/kernel/irq.c:358:2: note: in expansion of macro ‘this_cpu_write’
358 | this_cpu_write(posted_interrupt_desc.nv, POSTED_MSI_NOTIFICATION_VECTOR);
| ^~~~~~~~~~~~~~
./include/linux/percpu-defs.h:364:15: error: ‘sizeof’ applied to a bit-field
364 | switch(sizeof(variable)) { \
>
> > +static inline bool pi_test_and_set_on(struct pi_desc *pi_desc)
> > +{
> > + return test_and_set_bit(POSTED_INTR_ON,
> > + (unsigned long *)&pi_desc->control);
>
> Please get rid of those line breaks.
will do.
Thanks,
Jacob
On Thu, Dec 07 2023 at 20:54, Jacob Pan wrote: > On Wed, 06 Dec 2023 17:33:28 +0100, Thomas Gleixner <tglx@linutronix.de> > wrote: >> On Sat, Nov 11 2023 at 20:16, Jacob Pan wrote: >> u32 rsvd[6]; >> } __aligned(64); >> > It seems bit-fields cannot pass type check. I got these compile error. And why are you telling me that instead if simply fixing it?
Hi Thomas,
On Fri, 08 Dec 2023 10:31:20 +0100, Thomas Gleixner <tglx@linutronix.de>
wrote:
> On Thu, Dec 07 2023 at 20:54, Jacob Pan wrote:
> > On Wed, 06 Dec 2023 17:33:28 +0100, Thomas Gleixner <tglx@linutronix.de>
> > wrote:
> >> On Sat, Nov 11 2023 at 20:16, Jacob Pan wrote:
> >> u32 rsvd[6];
> >> } __aligned(64);
> >>
> > It seems bit-fields cannot pass type check. I got these compile error.
>
> And why are you telling me that instead if simply fixing it?
I guess we can fix it like this to use the new bitfields:
void intel_posted_msi_init(void)
{
- this_cpu_write(posted_interrupt_desc.nv, POSTED_MSI_NOTIFICATION_VECTOR);
- this_cpu_write(posted_interrupt_desc.ndst, this_cpu_read(x86_cpu_to_apicid));
+ struct pi_desc *pid = this_cpu_ptr(&posted_interrupt_desc);
+
+ pid->nv = POSTED_MSI_NOTIFICATION_VECTOR;
+ pid->ndst = this_cpu_read(x86_cpu_to_apicid);
It is init time, no IOMMU posting yet. So no need for atomics.
Thanks,
Jacob
Hi Thomas, On Fri, 08 Dec 2023 10:31:20 +0100, Thomas Gleixner <tglx@linutronix.de> wrote: > On Thu, Dec 07 2023 at 20:54, Jacob Pan wrote: > > On Wed, 06 Dec 2023 17:33:28 +0100, Thomas Gleixner <tglx@linutronix.de> > > wrote: > >> On Sat, Nov 11 2023 at 20:16, Jacob Pan wrote: > >> u32 rsvd[6]; > >> } __aligned(64); > >> > > It seems bit-fields cannot pass type check. I got these compile error. > > And why are you telling me that instead if simply fixing it? My point is that I am not sure this change is worthwhile unless I don't do the per CPU pointer check. gcc cannot take bit-field address afaik. So the problem is that with this bit-field change we don't have individual members anymore to check pointers. e.g. ./include/linux/percpu-defs.h:363:20: error: cannot take address of bit-field ‘nv’ 363 | __verify_pcpu_ptr(&(variable)); Thanks, Jacob
© 2016 - 2025 Red Hat, Inc.