Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5773db7f639871e840247579fc9a7835277ede74.camel@intel.com>
Subject: Re: [PATCH v8 2/5] x86/tdx: Add TDX Guest event notify interrupt
 support
From:   Kai Huang <kai.huang@intel.com>
To:     Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        "Yao, Jiewen" <jiewen.yao@intel.com>,
        "Nakajima, Jun" <jun.nakajima@intel.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "x86@kernel.org" <x86@kernel.org>,
        "H . Peter Anvin" <hpa@zytor.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        "Luck, Tony" <tony.luck@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Wander Lairson Costa <wander@redhat.com>,
        Isaku Yamahata <isaku.yamahata@gmail.com>,
        "marcelo.cerri@canonical.com" <marcelo.cerri@canonical.com>,
        "tim.gardner@canonical.com" <tim.gardner@canonical.com>,
        "khalid.elmously@canonical.com" <khalid.elmously@canonical.com>,
        "Cox, Philip" <philip.cox@canonical.com>,
        LKML <linux-kernel@vger.kernel.org>
Date:   Fri, 15 Jul 2022 11:58:06 +1200
In-Reply-To: <4421372e-14b5-1c6d-e4bf-5a88a2c88ab8@linux.intel.com>
References: <20220609025220.2615197-1-sathyanarayanan.kuppuswamy@linux.intel.com>
         <20220609025220.2615197-3-sathyanarayanan.kuppuswamy@linux.intel.com>
         <78873cc1db47ba00a4c01f38290521c1a6072820.camel@intel.com>
         <efb2cdab-289b-8757-fe5e-5348519b0474@linux.intel.com>
         <385B219C-4DB2-480C-913C-411AB4D644ED@intel.com>
         <MW4PR11MB5872E3B775A680678331D6358CB79@MW4PR11MB5872.namprd11.prod.outlook.com>
         <9c7d99469179340eeecabaf3e9c414fc98900626.camel@intel.com>
         <503e7135-782c-b72b-6f55-3c4acf55921b@linux.intel.com>
         <434ff0edcd5a0f1eb671bb2850ef5444ac1359a3.camel@intel.com>
         <4421372e-14b5-1c6d-e4bf-5a88a2c88ab8@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.3 (3.44.3-1.fc36) 
MIME-Version: 1.0
Precedence: bulk


> > > +/*
> > > + * Handler used to report notifications about
> > > + * TDX_GUEST_EVENT_NOTIFY_VECTOR IRQ. Currently it will be
> > > + * used only by the attestation driver. So, race condition
> > > + * with read/write operation is not considered.
> > > + */
> > > +static void (*tdx_event_notify_handler)(void);
> > > +
> > > +/* Helper function to register tdx_event_notify_handler */
> > > +void tdx_setup_ev_notify_handler(void (*handler)(void))
> > > +{
> > > +       tdx_event_notify_handler =3D handler;
> > > +}
> > > +
> > > +/* Helper function to unregister tdx_event_notify_handler */
> > > +void tdx_remove_ev_notify_handler(void)
> > > +{
> > > +       tdx_event_notify_handler =3D NULL;
> > > +}
> > > +
> >=20
> > Looks it's weird that you still need it.  Couldn't we pass the function=
 to
> > handle GetQuote directly to request_irq()?
>=20
>=20
> Since event notification is not only specific to attestation, I did not w=
ant to
> directly tie it to GetQuote handler.
>=20
> Alternative approach is, to make this IRQ vector shared and let its users=
 do
> request_irq() and free_irq() directly.

I think IRQF_SHARED is the logic.  We don't need it now as for now we have =
only
one user, though.

The "multiple users" case may never happen, so doesn't justify the addition=
al
'tdx_event_notify_handler' code at this moment.

>=20
> Something like below?
>=20
> --- a/arch/x86/coco/tdx/attest.c
> +++ b/arch/x86/coco/tdx/attest.c
> @@ -399,7 +399,7 @@ static const struct file_operations tdx_attest_fops =
=3D {
> =20
>  static int __init tdx_attestation_init(void)
>  {
> -       int ret;
> +       int ret, irq;
> =20
>         /* Make sure we are in a valid TDX platform */
>         if (!cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
> @@ -422,6 +422,14 @@ static int __init tdx_attestation_init(void)
>                 return ret;
>         }
> =20
> +       irq =3D tdx_get_evirq();

The function seems unnecessary.  You can put 'extern int tdx_evtirq;' to
asm/tdx.h if needed and use it directly.