linux-kernel - Re: [PATCH 5/5] x86/kvm: Add KVM_FEATURE_MSI_EXT_DEST

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: <0de733f6384874d68afba2606119d0d9b1e8b34e.camel@infradead.org>
Date:   Tue, 13 Oct 2020 08:52:04 +0100
From:   David Woodhouse <dwmw2@...radead.org>
To:     Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org,
        Marc Zyngier <maz@...nel.org>
Cc:     kvm <kvm@...r.kernel.org>, Paolo Bonzini <pbonzini@...hat.com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/5] x86/kvm: Add KVM_FEATURE_MSI_EXT_DEST_ID

On Tue, 2020-10-13 at 00:13 +0200, Thomas Gleixner wrote:
> On Mon, Oct 12 2020 at 21:20, David Woodhouse wrote:
> > On Mon, 2020-10-12 at 20:38 +0200, Thomas Gleixner wrote:
> > > Nasty, but way better than what we have now. 
> > 
> > Want me to send that out in email or is the git tree enough for now?
> > 
> > I've cleaned it up a little and fixed a bug in the I/OAPIC error path.
> 
> Mail would be nice once you are confident with the pile.

After the next cup of coffee. Will send it as an incremental series on
top of my previous ext_dest_id series.

> > Still not entirely convinced about the apic->apic_id_valid(32768) thing
> > but it should work well enough, and doesn't require exporting any extra
> > state from apic.c that way.
> 
> Yeah, that part is odd.
> 
> I really dislike the way how irq_find_matching_fwspec() works. The 'rc'
> value is actually boolean despite being type 'int' and if 'rc' is not 0
> then it returns the domain even if 'rc' is an error code.
> 
> But that does not allow to return error codes from a domain match() /
> select() callback which is what we really want to express that there is
> something fishy.

I don't know that we need to return an explicit error. I made
x86_vector_select() refuse to match if IRQ remapping is enabled, which
means that the irq_find_matching_fwspec() call will fail by returning
NULL in precisely the cases it should. (Which should never happen; qv).

That failure means HPET will refuse to set up MSI, or I/OAPIC will
refuse to initialise (later causing a BUG and a failure to boot, which
is probably the correct thing to do).

It's all fine.

+       dom = irq_find_matching_fwspec(fwspec, DOMAIN_BUS_IR);
+       if (dom)
+               return IS_ERR(dom) ? NULL : dom;
+
+       return x86_vector_domain;
+}

Ick. There's no need for that.

Eliminating that awful "if not found then slip the x86_vector_domain in
as a special case" was the whole *point* of using
irq_find_matching_fwspec() in the first place.

Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (5174 bytes)