KVM Archive on lore.kernel.org
 help / color / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: David Laight <David.Laight@ACULAB.COM>,
	"x86@kernel.org" <x86@kernel.org>
Cc: kvm <kvm@vger.kernel.org>,
	"iommu@lists.linux-foundation.org"
	<iommu@lists.linux-foundation.org>,
	"joro@8bytes.org" <joro@8bytes.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
	"maz@misterjones.org" <maz@misterjones.org>,
	Dexuan Cui <decui@microsoft.com>
Subject: Re: [PATCH v3 17/35] x86/pci/xen: Use msi_msg shadow structs
Date: Sun, 25 Oct 2020 10:26:14 +0000
Message-ID: <a7ed5e550628083199e2747b8267c550689a368c.camel@infradead.org> (raw)
In-Reply-To: <3e69326016524d97bcdea35d0765cc68@AcuMS.aculab.com>


[-- Attachment #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 #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

  reply index

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-07 12:20 [PATCH 0/5] Fix x2apic enablement and allow up to 32768 CPUs without IR where supported David Woodhouse
2020-10-07 12:20 ` [PATCH 1/5] x86/apic: Fix x2apic enablement without interrupt remapping David Woodhouse
2020-10-07 12:20   ` [PATCH 2/5] x86/msi: Only use high bits of MSI address for DMAR unit David Woodhouse
2020-10-07 12:20   ` [PATCH 3/5] x86/ioapic: Handle Extended Destination ID field in RTE David Woodhouse
2020-10-08  9:12     ` Peter Zijlstra
2020-10-08 17:05       ` David Woodhouse
2020-10-08 11:41     ` Thomas Gleixner
2020-10-07 12:20   ` [PATCH 4/5] x86/apic: Support 15 bits of APIC ID in IOAPIC/MSI where available David Woodhouse
2020-10-08 11:54     ` Thomas Gleixner
2020-10-08 12:02       ` Thomas Gleixner
2020-10-08 13:00       ` David Woodhouse
2020-10-07 12:20   ` [PATCH 5/5] x86/kvm: Add KVM_FEATURE_MSI_EXT_DEST_ID David Woodhouse
2020-10-08 12:05     ` Thomas Gleixner
2020-10-08 12:55       ` David Woodhouse
2020-10-08 16:08         ` David Woodhouse
2020-10-08 21:14           ` Thomas Gleixner
2020-10-08 21:39             ` David Woodhouse
2020-10-08 23:27               ` Thomas Gleixner
2020-10-09  6:07                 ` David Woodhouse
2020-10-10 10:06                 ` David Woodhouse
2020-10-10 11:44                   ` Thomas Gleixner
2020-10-10 11:58                     ` David Woodhouse
2020-10-11 17:12                       ` Thomas Gleixner
2020-10-11 21:15                         ` David Woodhouse
2020-10-12  9:33                           ` Thomas Gleixner
2020-10-12 16:06                             ` David Woodhouse
2020-10-12 18:38                               ` Thomas Gleixner
2020-10-12 20:20                                 ` David Woodhouse
2020-10-12 22:13                                   ` Thomas Gleixner
2020-10-13  7:52                                     ` David Woodhouse
2020-10-13  8:11                                       ` [PATCH 0/9] Remove irq_remapping_get_irq_domain() David Woodhouse
2020-10-13  8:11                                         ` [PATCH 1/9] genirq/irqdomain: Implement get_name() method on irqchip fwnodes David Woodhouse
2020-10-13  8:11                                         ` [PATCH 2/9] x86/apic: Add select() method on vector irqdomain David Woodhouse
2020-10-13  8:11                                         ` [PATCH 3/9] iommu/amd: Implement select() method on remapping irqdomain David Woodhouse
2020-10-13  8:11                                         ` [PATCH 4/9] iommu/vt-d: " David Woodhouse
2020-10-13  8:11                                         ` [PATCH 5/9] iommu/hyper-v: " David Woodhouse
2020-10-13  8:11                                         ` [PATCH 6/9] x86/hpet: Use irq_find_matching_fwspec() to find " David Woodhouse
2020-10-13  8:11                                         ` [PATCH 7/9] x86/ioapic: " David Woodhouse
2020-10-13  8:11                                         ` [PATCH 8/9] x86: Kill all traces of irq_remapping_get_irq_domain() David Woodhouse
2020-10-13  8:11                                         ` [PATCH 9/9] iommu/vt-d: Simplify intel_irq_remapping_select() David Woodhouse
2020-10-13  9:28                                       ` [PATCH 5/5] x86/kvm: Add KVM_FEATURE_MSI_EXT_DEST_ID Thomas Gleixner
2020-10-13 10:15                                         ` David Woodhouse
2020-10-13 10:46                                         ` Thomas Gleixner
2020-10-13 10:53                                           ` David Woodhouse
2020-10-13 11:51                                             ` David Woodhouse
2020-10-13 12:40                                               ` Thomas Gleixner
2020-10-08 11:46   ` [PATCH 1/5] x86/apic: Fix x2apic enablement without interrupt remapping Thomas Gleixner
2020-10-09 10:46 ` [PATCH v2 0/8] Fix x2apic enablement and allow up to 32768 CPUs without IR where supported David Woodhouse
2020-10-09 10:46   ` [PATCH v2 1/8] x86/apic: Fix x2apic enablement without interrupt remapping David Woodhouse
2020-10-09 10:46   ` [PATCH v2 2/8] x86/msi: Only use high bits of MSI address for DMAR unit David Woodhouse
2020-10-09 10:46   ` [PATCH v2 3/8] x86/apic: Always provide irq_compose_msi_msg() method for vector domain David Woodhouse
2020-10-09 10:46   ` [PATCH v2 4/8] x86/ioapic: Handle Extended Destination ID field in RTE David Woodhouse
2020-10-09 10:46   ` [PATCH v2 5/8] x86/apic: Support 15 bits of APIC ID in MSI where available David Woodhouse
2020-10-09 10:46   ` [PATCH v2 6/8] x86/kvm: Add KVM_FEATURE_MSI_EXT_DEST_ID David Woodhouse
2020-10-09 10:46   ` [PATCH v2 7/8] x86/hpet: Move MSI support into hpet.c David Woodhouse
2020-10-09 10:46   ` [PATCH v2 8/8] x86/ioapic: Generate RTE directly from parent irqchip's MSI message David Woodhouse
2020-10-22 21:43     ` Thomas Gleixner
2020-10-22 22:10       ` Thomas Gleixner
2020-10-23 17:04         ` David Woodhouse
2020-10-23 10:10       ` David Woodhouse
2020-10-23 21:28         ` Thomas Gleixner
2020-10-24  8:26           ` David Woodhouse
2020-10-24  8:41             ` David Woodhouse
2020-10-24  9:13             ` Paolo Bonzini
2020-10-24 10:13               ` David Woodhouse
2020-10-24 12:44                 ` David Woodhouse
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
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=joro@8bytes.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

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git