linux-kernel - Re: [PATCH v8 2/5] x86/tdx: Add TDX Guest event notify interrupt support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5773db7f639871e840247579fc9a7835277ede74.camel@intel.com>
Date:   Fri, 15 Jul 2022 11:58:06 +1200
From:   Kai Huang <kai.huang@...el.com>
To:     Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "Yao, Jiewen" <jiewen.yao@...el.com>,
        "Nakajima, Jun" <jun.nakajima@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Wander Lairson Costa <wander@...hat.com>,
        Isaku Yamahata <isaku.yamahata@...il.com>,
        "marcelo.cerri@...onical.com" <marcelo.cerri@...onical.com>,
        "tim.gardner@...onical.com" <tim.gardner@...onical.com>,
        "khalid.elmously@...onical.com" <khalid.elmously@...onical.com>,
        "Cox, Philip" <philip.cox@...onical.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 2/5] x86/tdx: Add TDX Guest event notify interrupt
 support


> > > +/*
> > > + * 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 = handler;
> > > +}
> > > +
> > > +/* Helper function to unregister tdx_event_notify_handler */
> > > +void tdx_remove_ev_notify_handler(void)
> > > +{
> > > +       tdx_event_notify_handler = NULL;
> > > +}
> > > +
> > 
> > Looks it's weird that you still need it.  Couldn't we pass the function to
> > handle GetQuote directly to request_irq()?
> 
> 
> Since event notification is not only specific to attestation, I did not want to
> directly tie it to GetQuote handler.
> 
> 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 additional
'tdx_event_notify_handler' code at this moment.

> 
> Something like below?
> 
> --- 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 = {
>  
>  static int __init tdx_attestation_init(void)
>  {
> -       int ret;
> +       int ret, irq;
>  
>         /* 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;
>         }
>  
> +       irq = tdx_get_evirq();

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