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

[Igor Mammedov]

re-sending reply as something went wrong with headers (I suspect Daniel's name 
formatting)
and email got bounced back.

On Wed, 16 Mar 2022 14:31:33 +0000
David Woodhouse <dwmw2@infradead.org> wrote:

> 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.  

I was talking about EXT_DEST_ID kvm extension.
With current upstream guest kernel, user gets only "bad cpu" messages
and then go figure what's wrong with configuration or
simply hangs in case of Windows.

> > 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?  

or maybe we are enabling irq remapping by default now.
I'll try to check, if guest is actually brings all CPUs up.

> 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().  

if we are to keep iommu dependency then moving to x86_cpus_init()
isn't an option, it should be done at pc_machine_done() time.

in practice partial revert of your c1bb5418e to restore
iommu check including irq remapping.
In which case, do we still need kvm_enable_x2apic() check
you are adding here?