From: David Woodhouse <dwmw2@infradead.org>
To: David Laight <David.Laight@ACULAB.COM>,
"x86@kernel.org" <x86@kernel.org>
Cc: "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
kvm <kvm@vger.kernel.org>, Dexuan Cui <decui@microsoft.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
"iommu@lists.linux-foundation.org"
<iommu@lists.linux-foundation.org>,
"maz@misterjones.org" <maz@misterjones.org>,
Paolo Bonzini <pbonzini@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v3 17/35] x86/pci/xen: Use msi_msg shadow structs
Date: Sun, 25 Oct 2020 10:26:14 +0000 [thread overview]
Message-ID: <a7ed5e550628083199e2747b8267c550689a368c.camel@infradead.org> (raw)
In-Reply-To: <3e69326016524d97bcdea35d0765cc68@AcuMS.aculab.com>
[-- Attachment #1.1: Type: text/plain, Size: 4547 bytes --]
On Sun, 2020-10-25 at 09:49 +0000, David Laight wrote:
> Just looking at a random one of these patches...
>
> Does the compiler manage to optimise that reasonably?
> Or does it generate a lot of shifts and masks as each
> bitfield is set?
>
> The code generation for bitfields is often a lot worse
> that that for |= setting bits in a word.
Indeed, it appears to be utterly appalling. That was one of my
motivations for doing it with masks and shifts in the first place.
Because in ioapic_setup_msg_from_msi(), for example, these two are
consecutive in both source and destination:
entry->vector = msg.arch_data.vector;
entry->delivery_mode = msg.arch_data.delivery_mode;
And so are those:
entry->ir_format = msg.arch_addr_lo.dmar_format;
entry->ir_index_0_14 = msg.arch_addr_lo.dmar_index_0_14;
But GCC 7.5.0 here is doing them all separately...
00000000000011e0 <ioapic_setup_msg_from_msi>:
{
11e0: 53 push %rbx
11e1: 48 89 f3 mov %rsi,%rbx
11e4: 48 83 ec 18 sub $0x18,%rsp
irq_chip_compose_msi_msg(irq_data, &msg);
11e8: 48 89 e6 mov %rsp,%rsi
{
11eb: 65 48 8b 04 25 28 00 mov %gs:0x28,%rax
11f2: 00 00
11f4: 48 89 44 24 10 mov %rax,0x10(%rsp)
11f9: 31 c0 xor %eax,%eax
irq_chip_compose_msi_msg(irq_data, &msg);
11fb: e8 00 00 00 00 callq 1200 <ioapic_setup_msg_from_msi+0x20>
entry->vector = msg.arch_data.vector;
1200: 0f b6 44 24 08 movzbl 0x8(%rsp),%eax
entry->delivery_mode = msg.arch_data.delivery_mode;
1205: 0f b6 53 01 movzbl 0x1(%rbx),%edx
1209: 0f b6 74 24 09 movzbl 0x9(%rsp),%esi
entry->vector = msg.arch_data.vector;
120e: 88 03 mov %al,(%rbx)
entry->dest_mode_logical = msg.arch_addr_lo.dest_mode_logical;
1210: 0f b6 04 24 movzbl (%rsp),%eax
entry->delivery_mode = msg.arch_data.delivery_mode;
1214: 83 e2 f0 and $0xfffffff0,%edx
1217: 83 e6 07 and $0x7,%esi
entry->dest_mode_logical = msg.arch_addr_lo.dest_mode_logical;
121a: 09 f2 or %esi,%edx
121c: 8d 0c 00 lea (%rax,%rax,1),%ecx
entry->ir_format = msg.arch_addr_lo.dmar_format;
121f: c0 e8 04 shr $0x4,%al
entry->dest_mode_logical = msg.arch_addr_lo.dest_mode_logical;
1222: 83 e1 08 and $0x8,%ecx
1225: 09 ca or %ecx,%edx
1227: 88 53 01 mov %dl,0x1(%rbx)
entry->ir_format = msg.arch_addr_lo.dmar_format;
122a: 89 c2 mov %eax,%edx
entry->ir_index_0_14 = msg.arch_addr_lo.dmar_index_0_14;
122c: 8b 04 24 mov (%rsp),%eax
122f: 83 e2 01 and $0x1,%edx
1232: c1 e8 05 shr $0x5,%eax
1235: 66 25 ff 7f and $0x7fff,%ax
1239: 8d 0c 00 lea (%rax,%rax,1),%ecx
123c: 66 c1 e8 07 shr $0x7,%ax
1240: 88 43 07 mov %al,0x7(%rbx)
1243: 09 ca or %ecx,%edx
}
1245: 48 8b 44 24 10 mov 0x10(%rsp),%rax
124a: 65 48 33 04 25 28 00 xor %gs:0x28,%rax
1251: 00 00
entry->ir_index_0_14 = msg.arch_addr_lo.dmar_index_0_14;
1253: 88 53 06 mov %dl,0x6(%rbx)
}
1256: 75 06 jne 125e <ioapic_setup_msg_from_msi+0x7e>
1258: 48 83 c4 18 add $0x18,%rsp
125c: 5b pop %rbx
125d: c3 retq
125e: e8 00 00 00 00 callq 1263 <ioapic_setup_msg_from_msi+0x83>
1263: 0f 1f 00 nopl (%rax)
1266: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
126d: 00 00 00
Still, this isn't really a fast path so I'm content to file the GCC PR
for the really *obvious* misses and let it take its course.
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]
[-- Attachment #2: Type: text/plain, Size: 156 bytes --]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-10-25 10:26 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <e6601ff691afb3266e365a91e8b221179daf22c2.camel@infradead.org>
2020-10-24 21:35 ` [PATCH v3 00/35] Fix x2apic enablement and allow more CPUs, clean up I/OAPIC and MSI bitfields David Woodhouse
2020-10-24 21:35 ` [PATCH v3 01/35] x86/apic: Fix x2apic enablement without interrupt remapping David Woodhouse
2020-10-24 21:35 ` [PATCH v3 02/35] x86/msi: Only use high bits of MSI address for DMAR unit David Woodhouse
2020-10-24 21:35 ` [PATCH v3 03/35] x86/apic/uv: Fix inconsistent destination mode David Woodhouse
2020-10-24 21:35 ` [PATCH v3 04/35] x86/devicetree: Fix the ioapic interrupt type table David Woodhouse
2020-10-24 21:35 ` [PATCH v3 05/35] x86/apic: Cleanup delivery mode defines David Woodhouse
2020-10-24 21:35 ` [PATCH v3 06/35] x86/apic: Replace pointless apic::dest_logical usage David Woodhouse
2020-10-24 21:35 ` [PATCH v3 07/35] x86/apic: Get rid of apic::dest_logical David Woodhouse
2020-10-24 21:35 ` [PATCH v3 08/35] x86/apic: Cleanup destination mode David Woodhouse
2020-10-24 21:35 ` [PATCH v3 09/35] x86/apic: Always provide irq_compose_msi_msg() method for vector domain David Woodhouse
2020-10-24 21:35 ` [PATCH v3 10/35] x86/hpet: Move MSI support into hpet.c David Woodhouse
2020-10-24 21:35 ` [PATCH v3 11/35] genirq/msi: Allow shadow declarations of msi_msg::$member David Woodhouse
2020-10-24 21:35 ` [PATCH v3 12/35] x86/msi: Provide msi message shadow structs David Woodhouse
2022-04-06 8:36 ` Reto Buerki
2022-04-06 8:36 ` [PATCH] x86/msi: Fix msi message data shadow struct Reto Buerki
2022-04-06 22:11 ` Thomas Gleixner
2022-04-07 11:06 ` Reto Buerki
2022-04-06 22:07 ` [PATCH v3 12/35] x86/msi: Provide msi message shadow structs Thomas Gleixner
2020-10-24 21:35 ` [PATCH v3 13/35] iommu/intel: Use msi_msg " David Woodhouse
2020-10-24 21:35 ` [PATCH v3 14/35] iommu/amd: " David Woodhouse
2020-10-24 21:35 ` [PATCH v3 15/35] PCI: vmd: " David Woodhouse
2020-10-28 20:49 ` Kees Cook
2020-10-28 21:13 ` Thomas Gleixner
2020-10-28 23:22 ` Kees Cook
2020-10-24 21:35 ` [PATCH v3 16/35] x86/kvm: " David Woodhouse
2020-10-24 21:35 ` [PATCH v3 17/35] x86/pci/xen: " David Woodhouse
2020-10-25 9:49 ` David Laight
2020-10-25 10:26 ` David Woodhouse [this message]
2020-10-25 13:20 ` David Laight
2020-10-24 21:35 ` [PATCH v3 18/35] x86/msi: Remove msidef.h David Woodhouse
2020-10-24 21:35 ` [PATCH v3 19/35] x86/io_apic: Cleanup trigger/polarity helpers David Woodhouse
2020-11-10 6:31 ` Qian Cai
2020-11-10 8:59 ` David Woodhouse
2020-11-10 16:26 ` Paolo Bonzini
2020-10-24 21:35 ` [PATCH v3 20/35] x86/ioapic: Cleanup IO/APIC route entry structs David Woodhouse
2020-10-24 21:35 ` [PATCH v3 21/35] x86/ioapic: Generate RTE directly from parent irqchip's MSI message David Woodhouse
2020-10-24 21:35 ` [PATCH v3 22/35] genirq/irqdomain: Implement get_name() method on irqchip fwnodes David Woodhouse
2020-10-25 9:41 ` Marc Zyngier
2020-10-24 21:35 ` [PATCH v3 23/35] x86/apic: Add select() method on vector irqdomain David Woodhouse
2020-10-24 21:35 ` [PATCH v3 24/35] iommu/amd: Implement select() method on remapping irqdomain David Woodhouse
2020-10-24 21:35 ` [PATCH v3 25/35] iommu/vt-d: " David Woodhouse
2020-10-24 21:35 ` [PATCH v3 26/35] iommu/hyper-v: " David Woodhouse
2020-10-24 21:35 ` [PATCH v3 27/35] x86/hpet: Use irq_find_matching_fwspec() to find " David Woodhouse
2020-10-24 21:35 ` [PATCH v3 28/35] x86/ioapic: " David Woodhouse
2020-10-24 21:35 ` [PATCH v3 29/35] x86: Kill all traces of irq_remapping_get_irq_domain() David Woodhouse
2020-10-24 21:35 ` [PATCH v3 30/35] iommu/vt-d: Simplify intel_irq_remapping_select() David Woodhouse
2020-10-24 21:35 ` [PATCH v3 31/35] x86/ioapic: Handle Extended Destination ID field in RTE David Woodhouse
2020-10-24 21:35 ` [PATCH v3 32/35] x86/apic: Support 15 bits of APIC ID in MSI where available David Woodhouse
2020-10-24 21:35 ` [PATCH v3 33/35] iommu/hyper-v: Disable IRQ pseudo-remapping if 15 bit APIC IDs are available David Woodhouse
2020-10-24 21:35 ` [PATCH v3 34/35] x86/kvm: Reserve KVM_FEATURE_MSI_EXT_DEST_ID David Woodhouse
2020-10-24 21:35 ` [PATCH v3 35/35] x86/kvm: Enable 15-bit extension when KVM_FEATURE_MSI_EXT_DEST_ID detected David Woodhouse
2020-10-25 8:12 ` [PATCH v3 00/35] Fix x2apic enablement and allow more CPUs, clean up I/OAPIC and MSI bitfields David Woodhouse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a7ed5e550628083199e2747b8267c550689a368c.camel@infradead.org \
--to=dwmw2@infradead.org \
--cc=David.Laight@ACULAB.COM \
--cc=decui@microsoft.com \
--cc=iommu@lists.linux-foundation.org \
--cc=kvm@vger.kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@misterjones.org \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).