Re: [PATCH 1/4] target/i386: Fix sanity check on max APIC ID / X2APIC enablement

[David Woodhouse]

On Wed, 2022-03-16 at 12:28 +0100, Igor Mammedov wrote:
> Generally Daniel is right, as long as it's something that what real hardware
> supports. (usually it's job if upper layers which know what guest OS is used,
> and can tweak config based on that knowledge).
> 
> But it's virt only extension and none (tested with
>  Windows (hangs on boot),
>  Linux (brings up only first 255 cpus)
> ) of mainline OSes ended up up working as expected (i.e. user asked for this
> many CPUs but can't really use them as expected).

As I said, that kind of failure mode will happen even with the split
irq chip and EXT_DEST_ID, with Windows and older (pre-5.10) Linux
kernels.

For older guests it would also happen on real hardware, and in VMs
where you expose an IOMMU with interrupt remapping. Some guests don't
support interrupt remapping, or don't support X2APIC at all.

> Which would just lead to users reporting (obscure) bugs.

It's not virt only. This could happen with real hardware.

> Testing shows, Windows (2019 and 2004 build) doesn't work (at least with
> just kernel-irqchip=on in current state). (CCing Vitaly, he might know
> if Windows might work and under what conditions)
> 
> Linux(recentish) was able to bring up all CPUs with APICID above 255
> with 'split' irqchip and without iommu present (at least it boots to
> command prompt).

That'll be using the EXT_DEST_ID support.

> What worked for both OSes (full boot), was split irqchip + iommu
> (even without irq remapping, but I haven't tested with older guests
> so irq remapping might be required anyways).

Hm, that's surprising for Windows unless it's learned to use the
EXT_DEST_ID support. Or maybe it *can* cope with only targeting
external interrupts at CPUs < 255 but has a gratuitous check that
prevents it bringing them up unless there's an IOMMU... *even* if that
IOMMU doesn't have irq remapping anyway?

Anyway, as fas as I'm concerned it doesn't matter very much whether we
insist on the split irq chip or not. Feel free to repost your patch
rebased on top of my fixes, which are also in my tree at
https://git.infradead.org/users/dwmw2/qemu.git

The check you're modifying has moved to x86_cpus_init().

smime.p7s
Description: S/MIME cryptographic signature