Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Jacob Pan <jacob.jun.pan@linux.intel.com>,
        LKML <linux-kernel@vger.kernel.org>, X86 Kernel <x86@kernel.org>,
        iommu@lists.linux.dev, Lu Baolu <baolu.lu@linux.intel.com>,
        kvm@vger.kernel.org, Dave Hansen <dave.hansen@intel.com>,
        Joerg Roedel <joro@8bytes.org>,
        "H. Peter Anvin" <hpa@zytor.com>, Borislav Petkov <bp@alien8.de>,
        Ingo Molnar <mingo@redhat.com>
Cc:     Raj Ashok <ashok.raj@intel.com>,
        "Tian, Kevin" <kevin.tian@intel.com>, maz@kernel.org,
        peterz@infradead.org, seanjc@google.com,
        Robin Murphy <robin.murphy@arm.com>,
        Jacob Pan <jacob.jun.pan@linux.intel.com>
Subject: Re: [PATCH RFC 01/13] x86: Move posted interrupt descriptor out of
 vmx code
In-Reply-To: <20231112041643.2868316-2-jacob.jun.pan@linux.intel.com>
References: <20231112041643.2868316-1-jacob.jun.pan@linux.intel.com>
 <20231112041643.2868316-2-jacob.jun.pan@linux.intel.com>
Date:   Wed, 06 Dec 2023 17:33:28 +0100
Message-ID: <87wmtruw87.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

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);

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