Linux-HyperV Archive on lore.kernel.org
 help / color / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: Thomas Gleixner <tglx@linutronix.de>, x86@kernel.org
Cc: kvm <kvm@vger.kernel.org>, Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-hyperv@vger.kernel.org, Dexuan Cui <decui@microsoft.com>
Subject: Re: [PATCH v2 8/8] x86/ioapic: Generate RTE directly from parent irqchip's MSI message
Date: Sat, 24 Oct 2020 09:26:13 +0100
Message-ID: <be564fccc341efa730b8cdfe18ef4d7e709ebf50.camel@infradead.org> (raw)
In-Reply-To: <87d01863a2.fsf@nanos.tec.linutronix.de>


[-- Attachment #1: Type: text/plain, Size: 3767 bytes --]

On Fri, 2020-10-23 at 23:28 +0200, Thomas Gleixner wrote:
> On Fri, Oct 23 2020 at 11:10, David Woodhouse wrote:
> > On 22 October 2020 22:43:52 BST, Thomas Gleixner <tglx@linutronix.de> wrote:
> > It makes the callers slightly more readable, not having to cast to uint32_t* from the struct.
> > 
> > I did ponder defining a new struct with bitfields named along the
> > lines of 'msi_addr_bits_19_to_4', but that seemed like overkill.
> 
> I did something like this in the meantime, because all of this just
> sucks.
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/apic
> 
> Hot of the press and completely untested.

Hm, your struct IO_APIC_route_entry isn't actually a union; you've
defined a 128-bit structure with the IR fields *following* the non-IR
fields. But there *is* a union in io_apic.c, of that 128-bit structure
and two uint32_ts. Suspect that wasn't quite what you intended. I'll
prod at it this morning and turn it into a single union of the three,
and give it some testing.

Also, my "Move MSI support into hpet.c" patch¹ got updated to
s/CONFIG_PCI_MSI/CONFIG_GENERIC_MSI_IRQ/ at about line 53 for the MSI-
related variable declarations, which was going to be in the next
version I posted.

I was also hoping Paolo was going to take the patch which just defines
the KVM_FEATURE_MSI_EXT_DEST_ID bit² ASAP, so that we end up with a
second patch³ that *just* wires it up to x86_init.msi_ext_dest_id() for
KVM.

¹ https://git.infradead.org/users/dwmw2/linux.git/commitdiff/734719c1f4
² https://git.infradead.org/users/dwmw2/linux.git/commitdiff/3f371d6749
³ https://git.infradead.org/users/dwmw2/linux.git/commitdiff/8399e14eb5

> Yes, we can't avoid the bit swizzling at all. But it can be made more
> readable.

Hm, I was about to concede that your version is a bit more readable.

But then I got to your new __ipi_msi_compose_msg() and realised that it
isn't working because it's setting the 0xFEE base address in the _low_
bits, somehow...

	msg->arch_addr_lo.base_address = X86_MSI_BASE_ADDRESS_LOW;
	printk("1 Compose MSI message %x/%x\n", msg->address_lo, msg->data);
	msg->arch_addr_lo.dest_mode_logical = apic->dest_mode_logical;
	printk("2 Compose MSI message %x/%x\n", msg->address_lo, msg->data);
	msg->arch_addr_lo.destid_0_7 = cfg->dest_apicid & 0xFF;
	printk("3 Compose MSI message %x/%x\n", msg->address_lo, msg->data);

[    1.793874] 1 Compose MSI message fee/0
[    1.794310] 2 Compose MSI message fee/0
[    1.794768] 3 Compose MSI message f02/0

And now I wish it was just a simple shift instead of an unholy maze of
overlapping unions of bitfields. But I'll make more coffee and stare at
it harder...

> Yes, that code is horrid, but adding a comment to that effect when
> changing it is not asked too much, right?

Sure. I just actually hadn't noticed that setting the dest/vector bits
right there was entirely redundant in the first place.

> I'm still wrapping my head around getting rid of this thing completely
> because now it's just a subset of your KVM case with the only
> restriction that I/O-APIC cannot be affined to any CPU with a APIC id
> greater than 255.

It was only ever that restriction anyway, wasn't it? Hyper-V PCI has
its own MSI handling, and there's no HPET so it was only ever the
I/OAPIC which was problematic there.

There are Hyper-V VM sizes with 416 vCPUs which depend on this today,
and which don't have the 15-bit MSI extension. Removing hyperv-iommu
would prevent us from using all the vCPUs on those. You *could* make
hyperv-iommu decline to initialise if x86_init.msi_ext_dest_id()
returns true though⁴.

⁴ https://git.infradead.org/users/dwmw2/linux.git/commitdiff/633ccf0d42

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

  reply index

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <803bb6b2212e65c568c84ff6882c2aa8a0ee03d5.camel@infradead.org>
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 [this message]
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
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=be564fccc341efa730b8cdfe18ef4d7e709ebf50.camel@infradead.org \
    --to=dwmw2@infradead.org \
    --cc=decui@microsoft.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.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

Linux-HyperV Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hyperv/0 linux-hyperv/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 linux-hyperv linux-hyperv/ https://lore.kernel.org/linux-hyperv \
		linux-hyperv@vger.kernel.org
	public-inbox-index linux-hyperv

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hyperv


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